opentracing-csharp icon indicating copy to clipboard operation
opentracing-csharp copied to clipboard

Should SpanContext use Java toTraceId/toSpanId signature?

Open austinlparker opened this issue 7 years ago • 2 comments

(Conversation forked from https://github.com/opentracing/specification/issues/123#issuecomment-429925629)

The recently-merged Trace Identifier RFC to the Java library chose to implement Trace Identifiers differently (see https://github.com/opentracing/opentracing-java/pull/280 for more) - essentially, rather than as a property on their span context, they add a toTraceId and toSpanId method. The rationale for this is to indicate that this action will perhaps cause memory allocations as Tracers would need to convert from their current in-memory representation to a string.

Should the C# library change as well?

austinlparker avatar Oct 15 '18 18:10 austinlparker

I have mixed feelings about this. It's important to stick to the spec but it's also important to not break stuff for "trivial" reasons - especially on the instrumentation/app side.

While the "to"-prefix in Java definitely indicates the potential allocations, it was also chosen (AFAIK) because existing tracers already used the initially proposed "traceId" name. We do not have this problem in C#.

So I'm undecided and looking forward to other opinions!

cwe1ss avatar Oct 15 '18 19:10 cwe1ss

My immediate thought is that we could easily extend the XML doc to indicate that calling TraceId/SpanId could trigger a conversion from whatever-to-string and may have performance impacts; We could also just note it in the README somewhere. Given that we're in technical compliance with the RFC as its written today, I'm generally against changing the interface but in favor of clarifying XML doc/general documentation comments.

austinlparker avatar Oct 15 '18 21:10 austinlparker