opentracing: diverging from spec about dd-trace-go HOT 9 CLOSED

This should all be fixed with the new release candidate that is in progress. StartSpanFromContext will take operation name that is set correctly. I don't see where this is set as a tag called resource.name in the code. It is however set as the default resource unless you specify the resource.name tag to override.

As for component, it should also be set correctly as a tag, like any other. Do note however that in the case of Datadog it might be best to use the span.type tag at least for now.

I'd hold of at least until the release candidate is out before switching to this API as there will be some changes. Afterwards bug reports and issues would be super encouraged, welcomed and helpful to us.

P.S. When you say "opentracing branch", what do you mean? Would be very helpful if your references to code would link to the code itself.

from dd-trace-go.

-- Heads-up --

We've approved (and soon it will be merged) the first step for this migration: #171

It will be part of our feature branch and not master, since we're planning to provide the release candidate from this branch. The next step is to update our integrations using this new API. Will provide a new update as soon we're there.

from dd-trace-go.

Hi, thanks for getting back to me.

I was referring to changes in #171 - looks like the release candidate you're talking about.

Would it be helpful for me to test with that branch?

from dd-trace-go.

Would it be helpful for me to test with that branch?

No, please don't. Not at all at this point. We are still breaking things and it's work in progress. I'd be very happy if you'd like to help us test once we finalise it, which should be soon.

from dd-trace-go.

@lwc we will provide a specific branch with all this work merged, so that we can ship a tag as a release candidate. Having your help to try out our new API, would be very appreciated! Also, we know that we're changing a lot of things in our API, but the end goal is to refactor the API so we can reach a very stable 1.0.

This is just to share or mid-term plan. Thank you for your details!

from dd-trace-go.

Hey, just looking over the v1 branch and it looks like this issue still exists:

The OpenTracing spec defines operation name as:

An operation name, a human-readable string which concisely represents the work done by the Span (for example, an RPC method name, a function name, or the name of a subtask or stage within a larger computation). The operation name should be the most general string that identifies a (statistically) interesting class of Span instances. That is, get_user is better than get_user/314159.

Your OpenTracing shim passes operation name through to the internal tracer like so: https://github.com/DataDog/dd-trace-go/blob/v1/ddtrace/opentracer/tracer.go#L44

Which ends up here: https://github.com/DataDog/dd-trace-go/blob/v1/ddtrace/tracer/span.go#L29.
This internal version of Operation Name is what the OT spec calls the component:

The software package, framework, library, or module that generated the associated Span. E.g., "grpc", "django", "JDBI".

This is concerning, as it means the OT implementation in this library has a DataDog flavour that will make interoperability with existing OpenTracing libraries impossible.

from dd-trace-go.

@lwc could it be that you are confusing things a little bit? I don't think we have a problem with the operation name. What you refer to as component within the OpenTracing spec is in fact what we call "span type" in the Datadog tracer.

from dd-trace-go.

@lwc you might also notice that if you don't specifically set a resource name on a span, the operation name is used as the resource internally: https://github.com/DataDog/dd-trace-go/blob/v1/ddtrace/tracer/tracer.go#L206

Is this not the behavior you expect?

from dd-trace-go.

This issue has gone quite stale, and I don't see anything actionable here.
I am going to close the issue, but please reopen if there's something that needs to be done.

from dd-trace-go.