Editor.config will help to streamline using the project

I think it'd make sense to agree on what namespaces the project should use.

#1 seems to be the way to go, and if #3 gets merged we'll end up with OpenCensus in the repo.

Suggestion 1:

• We just rename every OpenCencus namespace to OpenTelemetry.

Any other ideas?

See original issue: census-instrumentation/opencensus-csharp#64

Reviewing API surface - IList is used when IEnumerable will suffice. For instance in Exporter handler and other places. IList allows manipulating elements - add, remove, etc. when this is absolutely not the intent.

Making separate method for Name will make the span name update more intentional:
open-telemetry/opentelemetry-java#27

See https://github.com/open-telemetry/opentelemetry-java/tree/master/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim for the Java implementation

Copy from census-instrumentation/opencensus-csharp#141

see how it's done in Java and specs

open-telemetry/opentelemetry-java#144

I'll have a PR for this shortly

See: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/tracing-api.md#span

DependencyCollector now has a hardcoded list of supported dependencies (http only for now):

namespace OpenCensus.Collector.Dependencies{
       // ...
       public DependenciesCollector(DependenciesCollectorOptions options, ITracer tracer, ISampler sampler, IPropagationComponent propagationComponent)
        {
            this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(
                new Dictionary<string, Func<ITracer, Func<HttpRequestMessage, ISampler>, ListenerHandler>>()
                {
                    { "HttpHandlerDiagnosticListener", (t, s) => new HttpHandlerDiagnosticListener(t, s, propagationComponent) } ,
                   // e.g. how do I add { "Microsoft.Azure.Sdk", (t, s) => new AzureSdkListener(t, sampler) }
                },
                tracer,
                x =>  {... });
            this.diagnosticSourceSubscriber.Subscribe();
        }

We should allow configuring generic sources by name and allowing to provide custom handlers.
This could be done via DependenciesCollectorOptions.

This is also useful when users need to remove default collectors.

Find protos here: https://github.com/open-telemetry/opentelemetry-java/tree/master/sdk/src/main/proto

Use ActivityTraceId and ActivitySpanId instead of the library-own definition of TraceId and SpanId. https://github.com/dotnet/corefx/blob/5f9f2e57e668fccbc68e8f9c77852b2befef7712/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs#L881-L1197

The only complication will be the need to take dependeny on DiagnosticsSource package that will only become stable in September. But it is aligned reasonably well with our schedule

open-telemetry/opentelemetry-java#31

Copied from census-instrumentation/opencensus-csharp#137

Oops

See open-telemetry/opentelemetry-specification#55

Original: census-instrumentation/opencensus-csharp#51

I am assuming this should work but it doesnt listen on anything related to port 9184 then, if I change it to localhost instead of 0.0.0.0 then it works. Is it a bug or intentional?

var exporter = new PrometheusExporter(
new PrometheusExporterOptions()
{
    Url = new Uri("http://0.0.0.0:9184/metrics/")
},
Stats.ViewManager);

exporter.Start();

There are only a few aggregation types currently supported by Prometheus exporter. Need to finalize all of them.

See https://github.com/census-instrumentation/opencensus-csharp/blob/d087855fb93440f678009173d3f0f4392b58ce52/src/OpenCensus.Exporter.Prometheus/Implementation/PrometheusMetricBuilder.cs#L342-L382 for list of TODOs

Instead of exporter accepting SpanData - Tracer should.

Tags support metadata. Specifically - TTL.

open-telemetry/opentelemetry-java#38

See open-telemetry/opentelemetry-java#26

From: census-instrumentation/opencensus-csharp#129

When someone wants to implement a new listener for the component we don't support out of the box, they should be able to leverage DiagnosticListener subscription, and perhaps property fetching.

The first step could be making OpenCensus.Collector.Dependencies.Common.ListenerHandler public.

One scenario for this feature: I'm implementing DiagnosticSource instrumentation for my component and want to test out how it will work with OpenCensus.

open-telemetry/opentelemetry-java#90

One option is to follow this instruction: https://abelsquidhead.com/index.php/2019/04/13/getting-code-coverage-info-for-a-net-core-app-in-azure-pipelines/

Original: census-instrumentation/opencensus-csharp#59

hi, there.
I'm trying to use your library. Thank you for it.

So, I found some inconsistency in the implementation of exporters.
Namely, 'Sum' aggregation in case of Prometheus converted to 'gauge', in case of StackDriver to CUMULATIVE, which is 'counter'.

The fun is that neither of both is totally right.
'Sum' can represent either 'counter' or 'gauge'. For instance, when we count peers or users currently online it is very convenient to use 'Sum' and, in this case, it should be converted to 'gauge' by all exporters because value can go up and down
The second case, when we count the amount of data sent. When we have just buffer length it is very convenient to use 'Sum', but in this case, a value will only grow and measure should be converted to 'counter'

So, I would propose to add a second type for 'Sum' aggregation. first will be converted to 'gauge' second to 'counter'

Rename this project:
https://github.com/open-telemetry/opentelemetry-dotnet/tree/master/src/OpenTelemetry.Exporter.Ocagent

Into something like OpenTelemetry.Exporter.Grpc. Maybe an alternative can be OpenTelemetry.Exporter.OpenTelemetryService or OpenTelemetry.Exporter.OpenTelemetryExporter

Java should be used as a reference.

We discussed naming in #5. We may need to revisit it:

1. Abstraction package have only a subset of classes and interfaces used in SDK. So not really an abstraction. Perhaps rename it to Api instead for consistency with Java?
2. Should we rename OpenTelemetry to OpenTelemetry.Sdk? It will be consistent with Java. But harder for end user to follow the success path of "import OpenTelemetry" and you all set.

Let's discuss naming here

Activity from DiagnosticsSource package is already used to propagate a context. Activity is being set and restore by the framework so OpenTelemetry can rely on it.

It will be great to switch OpenTelemetry to use Activity as a way to propagate state (current Span, SpanContext and tags) instead of simply relying on AsyncLocal

open-telemetry/opentelemetry-java#94

Original: census-instrumentation/opencensus-csharp#130

The scenario:

Azure APIs return server request is in HTTP response headers and we want to put it on the span created to trace this http call.
We may argue the validity of this scenario and this might change in the future with W3C adoption, current support processes rely on this.

Also, this scenario of enhancing telemetry based on arbitrary request properties is popular among ApplicationInsights users.

While HttpClient instrumentation with DiagnosticSource allows it easily (you have the whole response object), Http listener in opencensus does not capture anything custom and does not provide extensibility points.

We have several options here

1. Keep it as is. Ask users/library owners to implement Http listeners on their own which just enhance current span. This approach requires deep knowledge of DiagnosticSource, error-prone and extremely inefficient. It also requires special configurations done by users.
   This is a viable workaround for now.

2. Provide hooks through DependenciesCollectorOptions like

class DependenciesCollectorOptions
{
  //...
 public  Action<HttpRequestMessage request, HttpResponseMessage response, Span span> EnhanceSpan;
}

3. Provide a way to disable automatic instrumentation for certain calls that require special treatment and ask users/libraries to implement instrumentation themselves with OpenCensus or DiagnosticSource.

• Provide configuration (e.g. for certain HTTP Url do not instrument). The problem of this approach is that by default 2 spans are collected and headers are injected twice. Only when a proper configuration is applied, we have something working. It's also quite inefficient and hard to configure
• It might be possible to auto-detect the presence of another instrumentation, but it would require setting some additional tags on the Activity OR attributes on Span OR both because so far we can have any of them :( So we'll need to implement another AsyncLocal which first instrumentation will lock and the next one would not run is detects this 'lock'.

Any of above requires someone to reimplement the whole HTTP instrumentation to do something as simple as reading one custom header. And custom instrumentations would be error prone and likely would not produce high-quality data.

So I believe
(1) is workaround
(2) is a possible solution, but we might want to spend time designing better API

From: census-instrumentation/opencensus-csharp#133

Start Jaeger

docker run -d --name jaeger   -e COLLECTOR_ZIPKIN_HTTP_PORT=9411   -p 5775:5775/udp   -p 6831:6831/udp   -p 6832:6832/udp   -p 5778:5778   -p 16686:16686   -p 14268:14268   -p 9411:9411   jaegertracing/all-in-one:1.11

Run the example zipkin or anything alike and Jaeger will give http 400 back. I got this via Wireshark when using the example TestZipkin.cs

See https://github.com/open-telemetry/opentelemetry-java/blob/3cc0af53af6a50837cb4ffdc880ce56f837c8538/api/src/main/java/io/opentelemetry/trace/Span.java#L44

open-telemetry/opentelemetry-java#145

Types like ISampledSpanStoreSummary, needs to be moved out of API. For now - at least to move them to SDK. Later we may decide to move them to the specific packages like zPages or alike

Original: census-instrumentation/opencensus-csharp#65

In some environments opening separate port for Prometheus exporter will not work. Especially for sandboxed environments like Azure Web Apps. Re-using app's port and allow Prometheus to work as a middleware will help to enable those environments.

Original: census-instrumentation/opencensus-csharp#143

When running the sample for Prometheus (TestPrometheus.cs) Prometheus server reports invalid data format in:

# HELP my_org_views_video_sizeprocessed video size over time
my_org_views_video_size{my_org_keys_frontend="mobile-ios9.3.5"} 173189802,666667 1555493470457

Culture setting for my machine is pl-PL. Apparently there's a problem with a comma which separates decimal places of the value. The source of the problem is writing a double value by StreamWriter.Write(double) in PrometheusMetricBuilder.Write, which applies national settings. I suppose this should be replaced by something like:

writer.Write(m.Value.ToString("G", CultureInfo.InvariantCulture));

open-telemetry/opentelemetry-java#81

Hi,

I'm recently implementing OpenCensus in the CMS-type application our company is developing. Beside using listeners for incoming and outgoing HTTP connections, provided in OpenCensus.Collector.AspNetCore and OpenCensus.Collector.Dependencies packages, I've reused the same pattern for DiagnosticSources our code is instrumented with.

I have some doubts with regards to my understanding of the way in which collectors/listeners plug into span hierarchy. In these two files:

https://github.com/census-instrumentation/opencensus-csharp/blob/develop/src/OpenCensus.Collector.AspNetCore/Implementation/HttpInListener.cs
https://githhub.com/census-instrumentation/opencensus-csharp/blob/develop/src/OpenCensus.Collector.Dependencies/Implementation/HttpHandlerDiagnosticListener.cs

OnStopActivity handler closes active span by calling its End method. This however does not change Tracer.CurrentSpan, and therefore next started span becomes a child span of the closed one, instead of a following. Current span is correctly reset when IScope instance produced by SpanBuilder.StartScopedSpan is disposed (which I did in my own listener, adding an async local stack of scopes).

Is here some thought I don't understand or mechanism I don't see? I can understand that for incoming HTTP requests resetting Tracer.CurrentSpan may be not important, because they are top level in a (local) trace, but for outgoing connections correct setting of CurrentSpan seems to me important.

Cheers,
Jacek

Similar to open-telemetry/opentelemetry-java#120