Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create mock tracer #21

Closed
jcchavezs opened this issue Sep 23, 2017 · 20 comments · Fixed by #46
Closed

Create mock tracer #21

jcchavezs opened this issue Sep 23, 2017 · 20 comments · Fixed by #46
Milestone

Comments

@jcchavezs
Copy link
Collaborator

jcchavezs commented Sep 23, 2017

OpenTracing is a set of interfaces and that means that unit testing your components will relay on specific implementations. For example, I want to assert what the attributes or tags are set in a span as part of my unit testing, what you need is a Tracer that creates a Span which is possible to inspect (exposing all the attributes available to do assertions). Something like:

Some examples:

From @yurishkuro in #40

Having a mock tracer is useful for two reasons:

  • it allows unit testing of the instrumentation with only the opentracing dependency
  • implementing a mock tracer helps with ironing out the issues in the API itself, like Removes carriers. #38, as it serves as a reference implementation

Ping @beberlei @felixfbecker

@asasmoyo
Copy link

Hello, I would like to start my contribution here :)
Could you describe how the mock should look like? Or could you give a scenario where the mock is used?

@jcchavezs
Copy link
Collaborator Author

jcchavezs commented Nov 12, 2017

Great @asasmoyo, I just updated the description.

@taoso
Copy link

taoso commented Nov 13, 2017

@jcchavezs @asasmoyo @yurishkuro @beberlei @felixfbecker
Please look at the Mockery. The Mockery is not the first mock framework for PHP, nor the last one.

Create a mock tracer is very simple. And I don't think the opentracing-php should shift a mock tracer.

As we are using PHP, please think in PHP. Thank you.

@felixfbecker
Copy link
Contributor

I don't think this is PHP-specific, Mockery is very similar to Mockito in Java. Does opentracing-java have a mock tracer?

@yurishkuro
Copy link
Member

-1 on using Mockery. It does not help with the objective

implementing a mock tracer helps with ironing out the issues in the API itself, like #38, as it serves as a reference implementation

@asasmoyo
Copy link

Great! Thanks everyone for the information. I'll study the code and try to come up with something by weekend

@jcchavezs
Copy link
Collaborator Author

We don't need a mock framework since we don't want to set any expectation, we need a tracer that fulfills the expectations and that exercises the API and all its constraints (like this one). In addition as @yurishkuro by providing the mock tracer we have a reference for changes in the API and implementation references.

@jcchavezs
Copy link
Collaborator Author

@felixfbecker both java, golang and javascript have them.

@taoso
Copy link

taoso commented Nov 16, 2017

@jcchavezs @felixfbecker @asasmoyo @yurishkuro
Whether or not we need shift the mock tracer, it has no influence on the tracing API, hasn't it?

So I propose to drop this issue from the stable milestone. And we can make more discussion for the mock tracer and introduce them in the future v1.x.x release.

@yurishkuro
Copy link
Member

Before declaring the API stable/1.x it should be proven in actual use. Don't get hung up on the "mock" part, think of it as reference implementation. Also, are there real tracers that implement this API?

The other part of proving the API is to use it to instrument existing open source frameworks. I don't know the php ecosystem, but I assume there are equivalents to Django, Express, Dropwizard. Have any of oss frameworks been instrumented? If they have been, how was the instrumentation unit tested without a mock tracer?

@taoso
Copy link

taoso commented Nov 16, 2017

@yurishkuro Actually, there do be one implementation, without the mock tracer. Please see taoso/jaeger-php@b1820db

@taoso
Copy link

taoso commented Nov 16, 2017

@yurishkuro

If they have been, how was the instrumentation unit tested without a mock tracer?

They depend on our interface, not implementation, not even our mock tracer!

So how they run their unit test? They use util like Mockery to create mock object for our interface and run their test.

What I need to emphasize is that the tracer does not influence the API, they are just a certain implementation. May be we can create a new repo named after opentracing-php-mock to include the mock tracer code. But it will never influence the API.

@taoso
Copy link

taoso commented Nov 16, 2017

Could we @jcchavezs drop this issue from the stable milestone?

@jcchavezs
Copy link
Collaborator Author

I don't think so @lvht. Most of people involved in this discussion agrees we need this for the stable release.

@taoso
Copy link

taoso commented Nov 16, 2017

@jcchavezs

In my opinion, the stable release is a release with stable API (our interface).

I am also refactoring the taoso/jaeger-php@b1820db. What the jaeger-php really need is only the interfaces opentracing-php shifted. As a PHP developer, what I really wanted is a stable API.

The mock tracer is just an additional feature of the opentracing-php repo. Wether or not shift with a mock tracer has no influence on the stability of the API of the opentracing API(interface).

By the way, the opentracing-php may be the first PHP binding for the OT spec, but it must not be the last one or the only one. The mock server is opentracing-php only, not OT spec only.

@taoso
Copy link

taoso commented Nov 17, 2017

ping @jcchavezs

@jcchavezs
Copy link
Collaborator Author

@lvht I will not hurry the release just to ship specific implementations, as I said most of the people involved on this agree that this is something we need so this is something we need before the stable release.

@taoso
Copy link

taoso commented Nov 29, 2017

@jcchavezs The due date of stable milestone is coming. Would you like make a stable 1.0 release against the OT 1.0 or delay to waiting for the OT 2.0?

@taoso
Copy link

taoso commented Nov 30, 2017

ping @jcchavezs

1 similar comment
@taoso
Copy link

taoso commented Dec 1, 2017

ping @jcchavezs

jcchavezs added a commit that referenced this issue Feb 1, 2018
jcchavezs added a commit that referenced this issue Feb 1, 2018
jcchavezs added a commit that referenced this issue Feb 1, 2018
@jcchavezs jcchavezs mentioned this issue Feb 1, 2018
6 tasks
jcchavezs added a commit that referenced this issue Feb 1, 2018
jcchavezs added a commit that referenced this issue Mar 10, 2018
jcchavezs added a commit that referenced this issue Mar 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants