opentracing / opentracing-php

OpenTracing API for PHP
Apache License 2.0
506 stars 56 forks source link

Create mock tracer #21

Closed jcchavezs closed 6 years ago

jcchavezs commented 7 years ago

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 #38, as it serves as a reference implementation

Ping @beberlei @felixfbecker

asasmoyo commented 6 years ago

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 commented 6 years ago

Great @asasmoyo, I just updated the description.

taoso commented 6 years ago

@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 commented 6 years ago

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

yurishkuro commented 6 years ago

-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 commented 6 years ago

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

jcchavezs commented 6 years ago

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 commented 6 years ago

@felixfbecker both java, golang and javascript have them.

taoso commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

@yurishkuro Actually, there do be one implementation, without the mock tracer. Please see https://github.com/lvht/jaeger-php/commit/b1820dbf7234a7369677baee24f1b9def98d9a71

taoso commented 6 years ago

@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 commented 6 years ago

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

jcchavezs commented 6 years ago

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

taoso commented 6 years ago

@jcchavezs

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

I am also refactoring the https://github.com/lvht/jaeger-php/commit/b1820dbf7234a7369677baee24f1b9def98d9a71. 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 commented 6 years ago

ping @jcchavezs

jcchavezs commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

ping @jcchavezs

taoso commented 6 years ago

ping @jcchavezs