0.11 Release plan

[cwe1ss]

This is a meta issue to track/discuss what should be done in the next release. The tasks reflect my proposal, any feedback/help is more than welcome!

Parity with opentracing-api from Java

The main thing that's missing right now is the support for scopes. This will only add new types/members but it will still be a breaking change (for implementations) since it adds members to an existing interface (ITracer).

Apart from that, the C# version currently has some other differences. We want to be as close to the Java version as possible so we will only keep differences that make sense for C#/.NET. We will do this with #50 which basically replaces the whole codebase and is based on the current master of opentracing-java.

This PR will introduce the following changes to the current code:

• Log methods will use IDictionary instead of IEnumerable. There's a discussion in Use IReadonlyDictionary<TKey, TValue> instead of IEnumerable<KeyValuePair<TKey, TValue>> #44 about whether or not there should be other changes regarding this in a later version.
• We will remove the FollowsFrom() methods in ISpanBuilder as they currently don't exist in Java.
• We will use Java's Tag types instead of our simple Tags-type with constants.
• We will use Java's propagation types. Our types were slightly different.
• We will add support for the BINARY format. It will be based on Stream
  • I still don't know if that's correct since I don't understand how this format is used - see also Carrier type for binary format #7.
  • Edit: we decided not to add BINARY for now as there's also some discussion around it in Java.
• We will remove the NullTracer from the core API. We will follow Java's lead and add it as a NoopTracer to a different package.
  • The current implementation is not good as there's currently a namespace and a type called NullTracer so one has to write NullTracer.NullTracer.
• We will remove our exception types (OpenTracingException, UnsupportedFormatException) as they don't exist in Java (they were from an unmerged PR)

Changing these things obviously results in a ton of breaking changes but I guess the impact is still minimal and the benefit of being on par with Java outweighs it.

We will keep the following differences. Please comment if you want to remove any of these differences as well!

• We are using DateTimeOffset instead ticks for tracking timestamps. The reasons for this are described in DateTime vs DateTimeOffset vs long (ticks) for timestamps  #4 and Replace DateTime usage with DateTimeOffset #42.
• We have a OpenTracing.LogFields type and Java has a OpenTracing.Log.Fields type, so the namespace and type names are different.
  • I think that makes a lot more sense as it doesn't introduce a new namespace for one file. Also, the Log-method is in the "OpenTracing" namespace so there's no point in having the constants in a different namespace.

Tasks:

• Merge Migrate/copy opentracing-java's content #50
• Remove exception types Remove exception types OpenTracingException and UnsupportedFormatException #64

Enhancements / Additions

We don't yet have an equivalent for Java's additional packages "util", "mock" and "noop". We will implement all of these in one OpenTracing.Utils package to reduce the number of packages.

• Please comment if you want to have separate libraries!

Tasks:

• Implement a GlobalTracer (Add a static Trace Class #45, GlobalTracer ported from Java #61).
• Implement the necessary types for scopes. They will be different though as .NET has AsyncLocal. (AsyncLocal based scope manager (ported from Java) #63)
• Implement a MockTracer (added MockTracer and MockTracer.Tests projects #54).
• Implement a NoopTracer (NoopTracer ported from Java #57).

Making sure stuff works

We only have a few unit tests as most of the code is interfaces. However we definitely need to port the tests from Java's util-package and we need to write our own for our AsyncLocal implementation.

We should also confirm that the API can actually be used by using it somewhere. I don't know if the basictracer-packages are still used in other languages. If so, we should update basictracer-csharp. Otherwise I have some personal prototypes. Other projects that would be willing to test stuff would be appreciated.

Tasks:

• Add unit tests for Utils
• Verify API usability in a project (e.g. basictracer-csharp)

Infrastructure

• People targeting the full framework currently get a lot of dependencies due to NETStandard.Library. We should either move to "netstandard10;net45" or even better "netstandard20;net45" (Target both netstandard1.0 and net45 to remove unneeded dependencies #51, Targets netstandard1.0 and net45 and updates build infrastructure to latest .NET Core SDK #56, Target netstandard1.3 and net45 #62).
• The build no longer works for me with 2.* SDKs of .NET core since some CLI commands have changed. (Targets netstandard1.0 and net45 and updates build infrastructure to latest .NET Core SDK #56)

Schedule

I've got some time next week, so I'd like to have PRs and discussions for the core API as far as possible by the end of next week. I can also create the utils-project with implementations for GlobalTracer and scopes. So depending on your feedback and the timeframe of other people who want to contribute, we might want to target a release between 02/15 and 02/28?!

[cwe1ss]

Added an "Infrastructure"-block.

[cwe1ss]

I rewrote the issue. It's now a proper plan as it says what will be changed and what will be kept.

This still reflects my proposal so please add your comments if you have any feedback!!!

[ndrwrbgs]

LGTM. I can start working on some of the boxes after #50 is in. [nit] SpanContextCorruptedException doesn't exist

I'd also add details regarding semantics.md, unless java has those implemented in it's Utils project in which it's covered (I just know my local copy I introduced methods for them).

[cwe1ss]

I'd also add details regarding semantics.md, unless java has those implemented in it's Utils project in which it's covered (I just know my local copy I introduced methods for them).

What do you mean by that?

[ndrwrbgs]

Semantic extensions on top of the specification

https://github.com/opentracing/specification/blob/master/semantic_conventions.md

E.g.

    public static class KnownTagNames
    {
        public const string Component = "component";

        public const string Error = "error";

        public static class Peer
        {
            public const string Port = "peer.port";
            public const string Address = "peer.address";
            public const string Hostname = "peer.hostname";
            public const string Ipv4 = "peer.ipv4";
            public const string Ipv6 = "peer.ipv6";
            public const string Service = "peer.service";
        }

        public static class Db
        {
            public const string Instance = "db.instance";
            public const string Statement = "db.statement";
            public const string Type = "db.type";
            public const string User = "db.user";
        }

        public static class Http
        {
            public const string Method = "http.method";
            public const string Url = "http.url";
            public const string StatusCode = "http.status_code";
        }

        public static class MessageBus
        {
            public const string Destination = "message_bus.destination";
        }

        public static class Span
        {
            public const string Kind = "span.kind";
        }

        public static class Sampling
        {
            public const string Priority = "sampling.priority";
        }

and structs like this for reporting those values easily

    public struct PeerData
    {
        public string Address { get; }
        public string HostName { get; }
        public string Ipv4 { get; }
        public string Ipv6 { get; }
        public int? Port { get; }
        public string Service { get; }

        public PeerData(string address, string hostName, string ipv4, string ipv6, int? port, string service)
        {
            this.Address = address;
            this.HostName = hostName;
            this.Ipv4 = ipv4;
            this.Ipv6 = ipv6;
            this.Port = port;
            this.Service = service;
        }
    }

[carlosalberto]

Other projects that would be willing to test stuff would be appreciated.

For the Span propagation part, we extracted and created a set of examples for trying out the approaches. All these examples are under the java-examples directory under opentracing-java, and I think we could take that approach, for a start (whenever we get to the point of implementing the IScopeManager ;) )

[carlosalberto]

We don't yet have an equivalent for Java's additional packages "util", "mock" and "noop". We will implement all of these in one OpenTracing.Utils package to reduce the number of packages.

I'm wondering about the motivation for this. The advantage over having different assemblies/packages, is that one of keeping resemblance between C# and Java (after all, they are kinda siblings). Guessing it's because of how packages are usually organized in the Nugget gallery?

Besides that, what I'm also curious about is the namespace organization - I think keeping similar namespaces to the Java packages would be good: i.e. OpenTracing.Mock, OpenTracing.NullTracer (or Noop), and OpenTracing.Util. Maybe that's altogether the plan already, but wanted to double check ;)

(I understand that usually C#/.Net has moved into having a few namespaces with lots of members, as opposed to Java and its multiple, sometimes deep nested packages with a few classes each, but in this case I keeping a similar structure with the Java API would be a good thing).

[cwe1ss]

@ndrwrbgs Semantic extensions on top of the specification

As it doesn't exist in Java (or not that I know of) I'd say we look into this after this release and discuss it with the Java folks. Ok?

[cwe1ss]

@carlosalberto: I'm wondering about the motivation for this. The advantage over having different assemblies/packages, is that one of keeping resemblance between C# and Java (after all, they are kinda siblings). Guessing it's because of how packages are usually organized in the Nugget gallery?

I'm not a fan of multiple packages if they don't have a clear architectural reason and I do not really see the reason to e.g. have different packages for Noop and Util as they will probably always be used together (Util has the scope stuff and GlobalTracer; and GlobalTracer depends on Noop). It's a lot easier for developers to discover new types within one package/assembly than it is to realize that one needs another package/assembly.

As none of these types (mock, util, noop) currently contain dependencies outside of the .NET standard library, I personally would even go as far as to include all of it as regular subfolders in the main OpenTracing assembly. Sure, the "OpenTracing" package is meant to be "the" abstraction library between instrumentation and implementation and a change to a util-class that is only necessary for instrumentation should not affect the implementation side but this would mean that packages would have different version numbers (e.g. "OpenTracing.Util" would be released more often than "OpenTracing") but that only leads to a version hell and makes it really hard for users/developers to know which packages work with each other. (e.g. OpenTracing.Util v0.14.0 depends on OpenTracing v0.11.0 and OpenTracing.Util v0.15.0 depends on OpenTracing v0.12.0, ...)

Anyway, as I've said, the proposal reflects my opinion (one has to start somewhere) and I'm more than happy to discuss this and to change the proposal!

So I see three possibilities:

• One OpenTracing package with namespaces/folders OpenTracing (ISpan, ...), OpenTracing.Noop, OpenTracing.Mock, OpenTracing.Util.
  • Advantage: All-in-One, easy to use
  • Disadvantages: Can't be released separately (if that's even a goal); once one of these packages needs a dependency outside of the .NET standard library, we need to introduce a different assembly; different than Java-version
• Separate packages for all of them
  • Advantage: Same as Java-version; Could be released separately
  • Disadvantage: Harder to use; version hell if they are released separately
• One OpenTracing assembly that equals opentracing-api from Java and one assembly for the rest (as in the current/initial proposal)
  • It's a compromise, so advantages and disadvantages are somewhere in between as well :)
  • Disadvantage: Assembly/Package names and namespaces should align so a "OpenTracing.Util" package should have "OpenTracing.Util.*" namespaces.

I would be OK with whatever the majority decides but given the fact that there's still only <= 5 active people here, I'd say that it's not really useful to decide based on votes/opinions. So I'd also be ok to just follow the "do whatever Java does" approach until the user base here is big enough to be able to better decide on whether or not a deviation from Java is actually useful/necessary. Therefore we could just go with separate assemblies!?

Thoughts?

[carlosalberto]

Hey Christian!

Thanks for jumping in :)

Disadvantage: Harder to use; version hell if they are released separately

Yeah, that needs some care - and that's why, even in the Java ecosystem, where each package has it's own artifact in Maven, they are all in the same repository, to prevent them from being released separately (while keeping their dependencies 'isolated', specially between the main api and noop/mock/util).

After a second look, I'd suggest a separated package that contains all the extra stuff (as you originally proposed), but without the Opentracing.Util. prefix for noop/mock (as that will create some de-alignment between Java and C# APIS, even if only mentally ;) ) Maybe for that to happen, this second package would need to be called something other than Util (Extra? Support?)

Else, as you also suggested, follow the Java path and deviate eventually as we go... (but still not sure, of course)

I would be OK with whatever the majority decides but given the fact that there's still only <= 5 active people here...

One of the things about being at the very beginning of a project :) Nevertheless, it's good the C# land has started to get some traction, little by little (finally ;) )

[gideonkorir]

@cwe1ss my vote is for 2 separate packages so long as they have the same version number (released simultaneously). As an implementer I only take a dependency on OpenTracing package but as a client/user I can decide to take a dependency on OpenTracing.Extras if I choose to.

@carlosalberto I agree with you on the util prefix plus the word util is confusing, is it a utility for using OpenTracing? something like Extras makes it known that it was built on top of the OpenTracing spec

If possible, I'd have the GlobalTracer in the OpenTracing namespace but still in the Extras package. This removes the need to keep typing in 2 usings (I know CTRL + . helps alleviate some of the pain here but still)

[cwe1ss]

I do not really like the names Extra(s), Support. I've never seen a popular package with a similar name on NuGet. Microsoft is using the term "Extensions" for their new "Microsoft.Extensions.*" packages which might sound better (at least to me)?!

Another thing to consider is that there will be OpenTracing.Contrib.* packages so having a OpenTracing.[Extras|Support|Extensions] package and e.g. a OpenTracing.Contrib package might also be confusing as one wouldn't know what contains what.

Since we don't have a proper name and since the namespaces should be the same as in Java, I now think that having a mixed package might be the worst option for now.

I would still prefer to have everything in one "OpenTracing" package as all of the code only uses BCL types and as that's also what other opentracing packages do (e.g. Go, PHP). This would also keep the same namespaces as the modules (Noop, Mock) would just be subfolders.

Thoughts? I hope I'll be available for the upcoming first cross-language working group meeting next week. If so, I'll discuss this there as well.

[cwe1ss]

BTW, the current release already has a Null/Noop tracer and it is part of the one „OpenTracing“ assembly.

[gideonkorir]

@cwe1ss just search Extras on nuget.org should return a few packages (Autofac has a lot of those).

Given that we only use BCL types and that we will always have to release in lock step, I change my mind - single Assembly makes most sense.

[cwe1ss]

I took a closer look at our exception types as they don't exist in Java. I created PR #64 with more details and updated this issue description. The PR would remove these types. Please post any comments in the PR.

[carlosalberto]

Since we don't have a proper name and since the namespaces should be the same as in Java, I now think that having a mixed package might be the worst option for now.

I agree on that, at least for now. I think one package could work just fine, as upon further inspection, it seems packages in Nuget do not get so 'split' as compared to Maven/Java.

[grounded042]

I've been tracking this project for a little while as the company I work for is looking to add tracing into our system. We're currently building out a proof of concept using version 0.10 of this lib. I'd be happy to help work towards a releasable 0.11. In the top-most post there are some work items - is there a specific one that isn't being worked on that would be good for me to start on? Or is there another area I could help?

[cwe1ss]

Everything has been merged so we are pretty close to a release IMO. I compared the code with Java once again and created a summary of everything that is different here (I hope I didn't miss anything):

General:

• We have everything in one package
  • Reason: Easier to manage, more common in .NET
• We use DateTimeOffset instead of long ticks
  • Reason: More common in .NET; there's not even an easy way to get the ticks without going through DateTime(Offset).

Core API:

• We have LogFields instead of log.Fields
  • Reason: There's not much point in a separate namespace for just one file
• ISpanBuilder is a first class citizen (vs Tracer.SpanBuilder in Java)
  • Reason: It's not common in C# to have public sub classes
• We don't have ISpanBuilder.StartManual()
  • Reason: It is already deprecated in Java
• We use finishSpanOnDispose instead of finishSpanOnClose
  • Reason: The pattern has a different name (Closable in Java vs Disposable in C#)
• Propagation: We use BuiltinFormats.* vs Format.Builtin.*
  • Reason: C# can't use types without constraints so we can't properly use sub classes on IFormat; also, since it's called IFormat it would be IFormat.Builtin which would be weird anyway.
• The property Key on the tags is public in C#
  • Reason: This allows people to use the actual string names and to do comparisons

Util:

• We don't have AutoFinishScope(Manager)
  • Reason: Not sure if we need it - we'll add it once somebody needs it.
• GlobalTracer throws if attempting to set itself; Java just writes a log message.
  • Reason: There is no common logging system in .NET

Noop:

• C# doesn't have interface fields so we don't use interfaces at all
• NoopSpanBuilder does NOT inherit NoopSpanContext
  • Reason: I think that's legacy. Still no response in Should NoopSpanBuilder still extend NoopSpanContext? opentracing-java#255

Mock:

• We have a separate Propagators type instead of members on the interface
  • Reason: Interface members are not possible in C#
• We renamed PrinterPropagator to ConsolePropagator
  • Reason: That's more common in .NET

I'll give people a few more days to provide feedback. If there's no objections, I'll release this beginning of next week!

In the meantime, I'm working on my ASP.NET Core integration to make sure we have a working example at the time of release.

/cc @bhs @tedsuo @yurishkuro

[MikeGoldsmith]

Looks good to me. Good work @cwe1ss @ndrwrbgs @Aaronontheweb

[Aaronontheweb]

One nitpick:

We use DateTimeOffset instead of long ticks

This is one change I'm not a fan of mostly because the native DateTime and DateTimeOffset are expensive and error-prone to serialize natively. For best results, you usually end up serializing them into their ticks representations anyway. I'd prefer that to be the native exposed format.

[cwe1ss]

@Aaronontheweb How would one get the ticks?

[Aaronontheweb]

https://github.com/akkadotnet/akka.net/blob/3f924fda8d6ca55c5af90a710d23be8ce36d5c7a/src/core/Akka/Util/MonotonicClock.cs

Seed the initial ticks value with DateTimeOffset.Now (instead of setting it to 0 like we do above) and then just add the delta provided by the Stopwatch running in the background.

edit: Or DateTimeOffset.UtcNow

[Aaronontheweb]

Like I said though, it's a nitpick.

[cwe1ss]

It's important to note that the overloads with a timestamp are mostly for replaying existing spans (e.g. from a log file) where people have a specific timestamp in the past (vs the current timestamp) so they have to parse/get the DateTime from somewhere anyway. So in this case it's all about usability.

Regular instrumentation will call the overloads without timestamps so tracers could optimize that path by using whatever mechanism they want to get the current timestamp.

[cwe1ss]

PS: Note that Stopwatch runs out of sync a little bit, that's why e.g. .NET corefx' Activity type synchronizes it every 2 hours. In .NET Core, DateTime uses high precision out of the box so they're no longer using Stopwatch there.

[grounded042]

I've got a working example over at https://github.com/Chatham/LetsTrace. There is an opentracing_11 branch which has the latest code from here integrated in. The library as a whole is still new and needs some good feedback, but I've tested it locally using the example.

[carlosalberto]

@cwe1ss I started porting (locally) the Java examples (from opentracing-java/opentracing-examples), and I expect to at least have a pair available prior to the release. Should be creating a PR over the next days ;)

[cwe1ss]

@carlosalberto that would be awesome!

[cwe1ss]

I've just released v0.11.0. I thought about creating a RC first but I don't think there's much point in it for this release as there's very few users anyway. We'll obviously change this in the upcoming releases.

Release notes: https://github.com/opentracing/opentracing-csharp/releases/tag/0.11.0

Please create a new issues with any problems/feedback.

Thanks everyone!

[bhs]

(@cwe1ss: congrats!!)