open-telemetry / opentelemetry-dotnet-contrib

This repository contains set of components extending functionality of the OpenTelemetry .NET SDK. Instrumentation libraries, exporters, and other components can find their home here.
https://opentelemetry.io
Apache License 2.0
463 stars 277 forks source link

B3Format extract creates an ActivityContext with default id when not passed in the request #1750

Open christopher-taormina-zocdoc opened 4 years ago

christopher-taormina-zocdoc commented 4 years ago

Describe your environment. Describe any aspect of your environment relevant to the problem:

I've set up the aspnetcore instrumentation on netcoreapp2.1 with the following in Startup.cs

services.AddOpenTelemetry((builder) => 
                    builder
                        .AddRequestInstrumentation((options) => { options.TextFormat = new B3Format(); })
                        .UseZipkinExporter((o) =>
                    {
                        o.ServiceName = "service-name;
                        o.Endpoint = new Uri("http://some-zipkin");
                    })
                );

While testing a request I noticed that the activity was given a SpanId of 0000000000000000 and TraceId of 00000000000000000000000000000000. It seems like when the HttpInListener detects a Request assembly with a major version < 3, the TextFormat option is used to pull the trace id's out from the http request headers and create a new ActivityContext. However, if these are not set in the request we end up with default id's of all 0's.

This seems to be caused by this line in the B3Format Extract function that returns RemoteInvalidContext.

I would expect these id's to be generated when not set on the HttpRequest, denoting a call made to an edge service and starting a new Trace.

Steps to reproduce. Setup an aspnetcore 2.1 server with the following in Startup.cs

services.AddOpenTelemetry((builder) => 
                    builder
                        .AddRequestInstrumentation((options) => { options.TextFormat = new B3Format(); })
                        .UseZipkinExporter((o) =>
                    {
                        o.ServiceName = "service-name;
                        o.Endpoint = new Uri("http://some-zipkin");
                    })
                );

Make an httpRequest to the server without the X-B3-TraceId and X-B3-SpanId headers sent.

See trace sent to zipkin with default SpanId and TraceId

What is the expected behavior? New TraceId and SpanId generated by the sdk.

What is the actual behavior? Default ids

eddynaka commented 4 years ago

@christopher-taormina-zocdoc , I debugged the application and yes, if you don't pass in the header X-B3-TraceId OR X-B3-SpanId, we will generate a Default value, so TraceId and SpanId will be zero.

@cijothomas , is that expected?

https://github.com/open-telemetry/opentelemetry-dotnet/blob/cbe99df381f1f1b8c05825b509486ad81dd9f3f2/src/OpenTelemetry/Context/Propagation/B3Format.cs#L185-L210