Step 1 - Use new Activity to Replace OT Span #660

cijothomas · 2020-05-04T17:03:00Z

Step1 of moving to use Activity to represent OT Spans. This is in alignment with the overall direction (https://medium.com/opentelemetry/opentelemetry-net-sdk-progress-3a63dcdc6cb0). There are some gaps between the OT Span API and .NET Activity API. They are captured in this issue : #675

This PR doesn't replace existing Spans, but instead coexists with existing, so everything working currently using Span/Tracer would work as is.
In a subsequent PR, all Span/Tracer would be removed, and replaced with Activity, ActivitySource - This will obviously be a breaking change (and a quite big one) for any existing early adopter customers.

Reference to Activity design:
dotnet/designs#98 this is the .NET Activity design proposals PR. This PR is based off on the designs explained in there.

What can be done once this change is in:

Configure OTel SDK to capture Activity from any number of sources.
Configure OTel SDK processing/exporting pipeline. ( A single pipeline only now for keeping changes minimal in step1)- i.e configure own ActivityProcessor and ActivityExporter
Use the provided ConsoleActivityExporter to view activities in Console.

Example instrumentation in Libraries:

var activitySource = new ActivitySource("MyCompany.MyProduct.MyLibraryName");
using (var activity = source.StartActivity("ActivityOrSpanName")) // if none listening, activity will be null.
{
	activity?.AddTag("tagname", "value");  //note the null check on activity
	// library code
} // activity gets Ended automatically on Dispose.

Example application code:

Enables activities from "MyCompany.MyProduct.MyWebServer" and uses ConsoleActivityExporter

OpenTelemetrySDK.EnableOpenTelemetry(
                (builder) => builder.AddActivitySource("MyCompany.MyProduct.MyWebServer")
                .UseConsoleActivityExporter(opt => opt.DisplayAsJson = options.DisplayAsJson));

Enables activities from "Source1", "Source2", "Source3" and uses custom processor and exporter.

OpenTelemetrySDK.EnableOpenTelemetry(
                (builder) => builder.AddActivitySource("Source1")
                .AddActivitySource("Source")
                .AddActivitySource("Source3")
                .SetProcessorPipeline(builder => builder.AddProcessor((proc) => new MyCustomProcessor(proc))
                .SetExporter(new MyCustomExporter())));

TestConsoleActivity shows a fully working example.

Next steps:
Port most logic of TracerBuilder to OpenTelemetryBuilder.- add batching, multiple pipeline support.
Respect Sampling decisions.
Add Jaeger exporter for Activity (may skip this and jump).
---breaking change now onwards----
Replace Span with Activity - would involve replacing all exporter and adapter code.....
Decide default behavior - should OT subscribe to all ActivitySources? Or selected ones? or All except disabled?

src/OpenTelemetry/Trace/Configuration/OpenTelemetrySDK.cs

reyang · 2020-05-04T17:16:37Z

src/OpenTelemetry/Trace/Configuration/OpenTelemetrySDK.cs

+        public static void EnableOpenTelemetry(string source, ActivityProcessor activityProcessor)
+        {
+            ActivitySource.AddActivityListener(
+            activitySource => activitySource.Name.Equals(source),


nit, indent.

reyang · 2020-05-04T17:20:22Z

src/OpenTelemetry/Trace/Export/SimpleActivityProcessor.cs

+                {
+                    try
+                    {
+                        disposableExporter.Dispose();


Looks like we're assuming that the exporter is only referenced by this processor (instead of being shared in multiple processors)?

current PR allows single pipeline only. But I think you are right, we cannot assume single ownership/dispose responsibility for exporters. I need to think this scenario and come back.

Thanks! Not a blocker for now.

pjanotti

Thanks for the preview @cijothomas - let's know when the NuGet package is available, we will have more useful feedback when dealing with the translations.

samples/Exporters/Console/Program.cs

samples/Exporters/Console/TestConsoleActivity.cs

src/OpenTelemetry/Trace/Configuration/OpenTelemetrySDK.cs

pjanotti · 2020-05-05T22:28:40Z

src/OpenTelemetry/Trace/Configuration/OpenTelemetrySDK.cs

+        public static void EnableOpenTelemetry(string source, ActivityProcessor activityProcessor)
+        {
+            ActivitySource.AddActivityListener(
+            activitySource => activitySource.Name.Equals(source),


@cijothomas crossing this with dotnet/runtime#35326 I can understand the parameters but it will be good to name the parameters or add comments here, especially for the second and third parameters. Anyway, the signatures are already better per https://source.dot.net/#System.Diagnostics.DiagnosticSource/System/Diagnostics/ActivityListener.cs,42a5d087d90a1839,references

reyang · 2020-05-06T00:16:21Z

Thanks for the preview @cijothomas - let's know when the NuGet package is available, we will have more useful feedback when dealing with the translations.

@pjanotti the current NuGet package is available as a single file System.Diagnostics.DiagnosticSource.5.0.0-dev.nupkg, maybe we can include in the PR and later clean it up (we might also consider rewrite git history if people hates binary files)?
Another option is to wait until May. 14th for the preview 4 release - that's the time we will have the NuGet from public feed.
I personally would recommend to move faster by including the binary file here (it is ~151KB).

pjanotti · 2020-05-06T16:08:53Z

I personally would recommend to move faster by including the binary file here (it is ~151KB).

@reyang I agree with that. @cijothomas can you add it to this PR?

cijothomas · 2020-05-06T16:37:15Z

I personally would recommend to move faster by including the binary file here (it is ~151KB).

@reyang I agree with that. @cijothomas can you add it to this PR?

Yes will do that. (packed day, so this would happen at end of day)

cijothomas · 2020-05-06T19:56:21Z

I personally would recommend to move faster by including the binary file here (it is ~151KB).

@reyang I agree with that. @cijothomas can you add it to this PR?

Yes will do that. (packed day, so this would happen at end of day)

Most likely we'll get the package from an alternate feed (like dotnet.myget...something) much earlier than may 14. So we wont have to checkin binaries. Waiting for official confirmation on this from .net team.

samples/Exporters/Console/TestConsoleActivity.cs

SergeyKanzhelev · 2020-05-06T20:03:25Z

samples/Exporters/Console/TestConsoleActivity.cs

+            // The following is generating activity.
+            // A library would simply write the following line of code.
+            var source = new ActivitySource("DemoSource");
+            using (var parent = source.StartActivity("parent"))


Is parent end up being display name or "enumeration" name?

If not a display name, please add code that sets the display name

StartActivity("anythinghere") - "anythinghere" becomes OperationName, DisplayName by default. DisplayName can be overridden, but OperationName cannot be.

I think the intention is DisplayName to become OT SpanName, as OT allows changing it after starting, but Activity.OperationName is fixed at creation.

SergeyKanzhelev · 2020-05-06T20:04:22Z

samples/Exporters/Console/TestConsoleActivity.cs

+                {
+                    child?.AddTag("child location", "child location");
+                }
+            }


If parent may be null (see parent?.), will the using statement fail?

No. Parent will be null if none is listening.

This is going to be a real pain, many many people will forgot the null check.

To sergey's qn - the using statement doesn't fail.
But any other place where parent is not nullchecked will fail. Hence the parent? usage throughout. Users are expected to do this, and this is unit testable easily.

samples/Exporters/Console/TestConsoleActivity.cs

SergeyKanzhelev · 2020-05-06T20:05:11Z

samples/Exporters/Console/TestConsoleActivity.cs

+            var source = new ActivitySource("DemoSource");
+            using (var parent = source.StartActivity("parent"))
+            {
+                parent?.AddTag("parent location", "parent location");


Are integers and other types allowed?

Only string - this is a drift from OT.

SergeyKanzhelev · 2020-05-06T20:06:12Z

samples/Exporters/Console/TestConsoleActivity.cs

+                using (var child = source.StartActivity("child"))
+                {
+                    child?.AddTag("child location", "child location");
+                }


please add example with the result setting. I bet the most of implementation will need try{}catch to indicate that the activity failed

…eed.

reyang · 2020-05-08T01:49:34Z

src/OpenTelemetry/Trace/Export/SimpleActivityProcessor.cs

+        {
+            try
+            {
+                // do not await, just start export


Curious, I think this is supposed to wait until exporter returned?

its fire and forget, so no await.

This seems to be different from other languages, for example Python.

Without a backpressure system couldn't this become an issue if the producer overloads ability to process these Tasks?
Perhaps at least we can keep track how how many Tasks are in-flight and drop items if it reaches a certain configurable limit?

yes, this deserve a revisit. I followed the model from SimpleSpanProcessor.

@bruno-garcia let me know if this is a blocker for this PR or not. I can address this in next step, if you have no objections.

Agree, without back pressure support we could easily over-produce and not understand where the blockage is. I think we need to at least have a plan for how we'd like to address it, even if the implementation comes at a later point.

This PR is not introducing this (I am just taking current SimpleSpanProcessor implementation) - These concerns/issues exist today, and is not addressed in this PR.

We definitely need to address this, and opening an issue to track this as this shouldn't block this PR :#674

Agree that we don't want to block this PR.

src/OpenTelemetry/Trace/Export/SimpleActivityProcessor.cs

reyang · 2020-05-08T01:55:08Z

src/OpenTelemetry/Trace/Export/SimpleActivityProcessor.cs

+        {
+            if (!this.disposed)
+            {
+                this.disposed = true;


This is not thread safe?

reyang · 2020-05-08T23:28:09Z

samples/Exporters/Console/TestConsoleActivity.cs

+                        }
+                    }
+
+                    parent?.AddTag("http.status_code", "200");


According to the spec it is an integer.

yup. Tags are currently string,string only. This is a drift from OT spec.

I think http.status_code should use parent?.SetCustomProperty("http.status_code", 200) rather than tag?

The current adapters use Attributes for storing status_code with ("http.status_code", 200). Attributes support int, but its replacement Activity.Tags only support string.

On top of this, Spans have Status, which don't have a place in Activity yet - the suggestion was to use activity.SetCustomProperty, for the SpanStatus. This was not finalized as well, as there was plans to drop the Status from Span?.

Agree this needs to be finalized - do we need it for this PR or we can do it when more clarity is achieved?
My simplest Exporter is not looking at activity.CustomProperties. Would add it in subsequent PR.

discussion in dotnet activity PR: https://github.com/dotnet/designs/pull/98/files#diff-25bba2a21bc64ddeccd374c1c28a0fefR406

Tags are currently string,string only. This is a drift from OT spec.

It will be good if exporters (at least the ones on this repo) have consistent behavior in this regard, eg.: by default have every value as a string and optionally enable a best-effort to convert to some "supported" types by the exporter format.

... as there was plans to drop the Status from Span?.

is anybody actively pursuing that?

If the discussion here regarding supporting the OT Status type, then SetCustomProperty can be used here. But if we are talking about adding simple HTTP status code, then I believe using Tags will be more reasonable as it will log a simple code and no further information attached to it (e.g. description or even the thrown exception).

For HTTP the existing semantic conventions are already enough (and even the OTel Status can be derived from that). For others wanting to set span status or equivalent, we need further conventions to do the same.

Is there any pending work to support non-string values as activity tags?

Is there any pending work to support non-string values as activity tags?

Yes. Tarek is working on proposals.

reyang

LGTM with some non-blocking issues that need to be revisited after this PR.

src/OpenTelemetry/Trace/Configuration/OpenTelemetryBuilder.cs

CodeBlanch · 2020-05-11T16:54:16Z

One thing that feels kind of off to me is...
builder.AddActivitySource("MyCompany.MyProduct.MyWebServer")
...introduces a "magic string" dependency. Not much we can do about it though, provide a class with well-known activity sources (constants) just to be slightly more helpful?

cijothomas · 2020-05-11T17:00:05Z

One thing that feels kind of off to me is...
builder.AddActivitySource("MyCompany.MyProduct.MyWebServer")
...introduces a "magic string" dependency. Not much we can do about it though, provide a class with well-known activity sources (constants) just to be slightly more helpful?

Yes.
One of the next step is:
Decide default behavior - should OT subscribe to all ActivitySources? Or selected ones? or All except disabled?

src/OpenTelemetry.Exporter.Console/ConsoleActivityExporter.cs

src/OpenTelemetry/Trace/Configuration/OpenTelemetryBuilder.cs

src/OpenTelemetry/Trace/Configuration/OpenTelemetrySDK.cs

bruno-garcia · 2020-05-11T17:10:47Z

src/OpenTelemetry/Trace/Export/SimpleActivityProcessor.cs

+        {
+            try
+            {
+                // do not await, just start export


Without a backpressure system couldn't this become an issue if the producer overloads ability to process these Tasks?
Perhaps at least we can keep track how how many Tasks are in-flight and drop items if it reaches a certain configurable limit?

MikeGoldsmith

I may have missed some of the conversation, but is this expected to supersede span / tracer for manual instrumentation? Is there any recorded discussion on this topic?

I find the ActivitySource API difficult to use and not a good match for the OpenTelemetry spec. I've highlighted some issues in my review.

If this was just to extend support for existing ActivitySource I think it would be okay as long as the main programming interface was spans and the Activities were translated to spans.

MikeGoldsmith · 2020-05-11T19:28:42Z

src/OpenTelemetry/Trace/Configuration/ActivityProcessorPipelineBuilder.cs

+        /// </summary>
+        /// <param name="processorFactory">Factory that creates exporting processor from the exporter.</param>
+        /// <returns>Returns <see cref="ActivityProcessorPipelineBuilder"/>.</returns>
+        public ActivityProcessorPipelineBuilder SetExportingProcessor(Func<ActivityExporter, ActivityProcessor> processorFactory)


Could we determine the 'exporting' processor is the last processor in the list instead of explicitly setting it?

This is the same behavior as current SpanProcessorPipelineBuilder - I'd defer any changes to current design in subsequent PRs to unblock progress on this PR.

MikeGoldsmith · 2020-05-11T19:30:52Z

samples/Exporters/Console/TestConsoleActivity.cs

+
+            // The below commented out line shows more likely code in a real world webserver.
+            // using (var parent = source.StartActivity("HttpIn", ActivityKind.Server, HttpContext.Request.Headers["traceparent"] ))
+            using (var parent = source.StartActivity("HttpIn", ActivityKind.Server))


I really do not like this API - it's very easy to missing a null check and introduce a non-obvious error. It also goes against the OpenTelemetry spec that a no-op tracer / span is returned in such a way no code needs to change.

it's very easy to missing a null check and introduce a non-obvious error.

StartActivity is not different than any other API that can return a null value. we use the ? for simplifying the code. Usually, users adopt new APIs will look at the doc and usage patterns. Also, usually, anyone writes code like that, is going to test it. simple case testing will catch this problem very easily.

It also goes against the OpenTelemetry spec that a no-op tracer/span is returned in such a way no code needs to change.

There is no code that needs to change anyway. right? The code will be the same either there are listeners or not. please notice, Activity is a type that is used for many years. In our design, we wanted to serve both users already using Activity and OT scenarios.

CC @noahfalk

Hey @tarekgh - I didn't mean my comments to come across so negative, sorry. I'll try to be more constructive.

I guess my main issue is that it is significantly different in usage compared to the OpenTelemetry specification. The span & tracer interfaces are designed to be robust and easy to use, providing an efficient no-op design if configuration is incomplete.

The Activity API can return null and must be checked for manually every time a usr wishes to interact with it, which becomes error prone if using the shorthand null check. It also does not support non-string tags (see http status code) which deviates from the spec and will cause propagation issues between language implementations (all other languages allow non-strings).

@MikeGoldsmith I appreciate your feedback, it is helpful. So, don't hesitate to send more :-)

Yes, I understand there are some differences between the OT specs and the .NET APIs. We have been working hard to get them close and fill the gaps but at the same time, we are restricted by some other constraints. We had to come with a design that not confuses a lot of developers who currently using the Activity APIs and at the same allow the OT scenarios. Think about it as we are not strictly implementing the OT spec as it is but we are enabling all scenarios that OT can do.

We'll continue to look at the feedback and try to figure out how we can help more with that. Thanks again for your feedback.

I didn't mean my comments to come across so negative

No worries at all @MikeGoldsmith, your initial feedback was straightforward and the objections were grounded. It was fine feedback : )

I guess my main issue is that it is significantly different in usage compared to the OpenTelemetry specification. The span & tracer interfaces are designed to be robust and easy to use, providing an efficient no-op design if configuration is incomplete

I agree, it is different. From my perspective deciding to reuse Activity APIs and integrate into the platform rather than creating a competing API has a lot of advantages, but one disadvantage is that it creates additional constraints on the design. In this case OT's premise is that it is easy and cheap to create a no-op Span so that nobody has to do a null check. We considered a no-op Activity, but didn't like the options:

If the no-op Activity behaves like a normal Activity then it is stateful and we can't use a global singleton. This means we are paying for allocations and all the normal initialization. Some of the work unfortunately isn't that cheap.

If the no-op Activity doesn't behave like a normal Activity we can make it cheap, but now we've created some risk that pre-existing code assets had assumptions about how Activity works which are invalidated. For example maybe the existing code assumes that properties can be read back after they are set, or that the timestamp of a started activity isn't 0.

So we ended at option 3, which was let the user see the null result. Our hope is that compiler nullable reference analysis makes it obvious when you've done the wrong thing, or if you are using an older compiler then code snippets, docs, and testing. Certainly its possible that this will be more problematic than we estimated so getting this kind of feedback is important (thanks!). So far we haven't heard much concern raised around this, but our overall feedback volume is probably still low.

It also does not support non-string tags

This one should probably be separated into its own issue. @cijothomas @reyang - have you guys experimented much with tags that would be non-string types in other languages? Certainly I see some potential for friction here.

This one should probably be separated into its own issue. @cijothomas @reyang - have you guys experimented much with tags that would be non-string types in other languages? Certainly I see some potential for friction here.

Yep. @cijothomas @tarekgh and I were chatting about this topic earlier today. We do see a good value of having non-string types.

MikeGoldsmith · 2020-05-11T19:31:35Z

samples/Exporters/Console/TestConsoleActivity.cs

+                        }
+                    }
+
+                    parent?.AddTag("http.status_code", "200");


Is there any pending work to support non-string values as activity tags?

MikeGoldsmith · 2020-05-11T19:32:17Z

samples/Exporters/Console/TestConsoleActivity.cs

+                {
+                    child?.AddTag("child location", "child location");
+                }
+            }


This is going to be a real pain, many many people will forgot the null check.

MikeGoldsmith · 2020-05-11T19:34:20Z

src/OpenTelemetry/Trace/Configuration/OpenTelemetrySdk.cs

@@ -0,0 +1,76 @@
+// <copyright file="OpenTelemetrySdk1.cs" company="OpenTelemetry Authors">


Suggested change

// <copyright file="OpenTelemetrySdk1.cs" company="OpenTelemetry Authors">

// <copyright file="OpenTelemetrySdk.cs" company="OpenTelemetry Authors">

This is going to be a real pain, many many people will forgot the null check

I don't think so. but anyway we'll watch the usage of the API.

Is there any pending work to support non-string values as activity tags?

No. Tags support strings only. OT support numbers/Bool which can still be used as strings.

MikeGoldsmith · 2020-05-11T19:37:27Z

src/OpenTelemetry/Trace/Export/SimpleActivityProcessor.cs

+        {
+            try
+            {
+                // do not await, just start export


Agree, without back pressure support we could easily over-produce and not understand where the blockage is. I think we need to at least have a plan for how we'd like to address it, even if the implementation comes at a later point.

johnduhart · 2020-05-12T20:40:19Z

I had a short discussion just before in the OTel .NET gitter channel, and I'm copying here as I felt @cijothomas's answers were helpful to me.

John Du Hart @johnduhart 16:10
I've been following this Span/Activity discussion for a bit and I wanted to echo some of @MikeGoldsmith's concerns. I'm not sure where to best put them given the number of different discussion in progress, so I'll share here.

Coming from the angle of a end-user to OTel, where I work we have services in a number of different languages maintained by other teams. While we've mostly achieved basic request tracing using Zipkin across our platform, I've been trying to champion more in-depth instrumentation with our services. One of the greatest draws of OTel (and previously OT) for me was its consistent model and nomenclature, such that I could explain concepts and usage to teams using languages I'm not too familiar with. Moving to a .NET specific API breaks this.

Have we also addressed if how switching to using Activity impacts the API/SDK separation outlined in the OTel specs, and if so is that an important thing to uphold? https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/library-guidelines.md#language-library-generic-design

Lastly I do have a question around if this is adding a dependency on .NET 5, is that the case? We have applications that need to stay on an LTS release of .NET Core, and right now that's 3.1.

Cijo Thomas @cijothomas 16:17
in reverse order. There is no dependency on .NET 5. The dependency is on System.Diagnostics.DiagnoticSource package which is shipped out of band - so all apps including .NET Framework 4.5, .NET Core 2.0 etc can use this. (The timing of DiagosticSource v5 release matches .NET 5 release, but that doesn't mean its tied to .NET 5 only) - A lot of folks asked this question and @ankit will be posted this as "FAQ"s very soon to the repo!
The API/SDK separation is maintained. For .NET the API is the BCL itself. For library authors simply write instrumentation against BCL (activity API).
this separation is one of the most important things
as libraries like HttpClient, etc which are part of .NET framework, will be able to take no additional dependency on anything, but the BCL to instrument themselves.
also - as Noah mentioned in the discussions in Github, a lot of .NET library authors (eg: every Azure library), will not take dependency on OpenTelemetry, they'll only take dependency on the .NET platform itself.

Cijo Thomas @cijothomas 16:26
@johnduhart thanks for reviewing this. Hope the above answers part of your concerns.
We'll be discussing more on this on SIG call today.(as we were doing for last 3-4 weeks) Please join if your schedule permits - if not we'll have recordings. And there will be Meeting notes, Github issues describing the outcomes of meeting.

Cijo Thomas @cijothomas 16:31
@johnduhart feel free to respond to the PR in Github as well - it may be easier for future members to follow Github discussion, than gitter channel.

John Du Hart @johnduhart 16:34
@cijothomas Thanks for responding. That's good to hear the changes are contained within the DiagnoticSourcepackage which we can consume out of band. And thank you for clarifying the API/SDK bit, the fact that the BCL becomes the API makes sense.

I can appreciate the concerns around framework dependencies. If library authors are not willing or able to take on additional dependencies, then that's an issue.

CodeBlanch · 2020-05-12T21:38:19Z

src/OpenTelemetry/Trace/Configuration/OpenTelemetrySdk.cs

+        /// <remarks>
+        /// Basic implementation only. Most logic from TracerBuilder will be ported here.
+        /// </remarks>
+        public static void EnableOpenTelemetry(Action<OpenTelemetryBuilder> configureOpenTelemetryBuilder)


Something feels a bit off about OpenTelemetry.Trace.Configuration.OpenTelemetrySdk.EnableOpenTelemetry. How would that look with both tracing and metrics? Would we need to do...

OpenTelemetry.Trace.Configuration.OpenTelemetrySdk.EnableOpenTelemetry(...); OpenTelemetry.Metrics.Configuration.OpenTelemetrySdk.EnableOpenTelemetry(...);

I'm thinking it should be more like...

OpenTelemetry.OpenTelemetrySdk .EnableTracing(...) .EnableMetrics(...)

Doesn't need to be done as part of this PR.

Not in this PR:

We probably want to consider moving Configuration outside Trace/Metrics namespace.

In Python it is called opentelemetry.sdk.

Agree. Something simpler like OpenTelemetry.Configuration or similar.

CodeBlanch · 2020-05-12T21:39:51Z

@cijothomas I left a few comments but LGTM.

SergeyKanzhelev · 2020-05-12T23:43:37Z

Notes of discussion are here: https://docs.google.com/document/d/1yjjD6aBcLxlRazYrawukDgrhZMObwHARJbB9glWdHj8/edit#

pjanotti

Per discussion on the SIG let's merge this PR and follow up on the discussion about the Trace API as a separate issue. Merging it makes it easier to experiment with ActivitySource and provide better feedback and decide about the API.

SergeyKanzhelev · 2020-05-13T00:09:41Z

Agree. Let's give @bruno-garcia and @MikeGoldsmith till tomorrow mid day to comment if there is something blocking

bruno-garcia

LGTM! We just need to keep in mind the fire and forget bits to address in the future. Thanks @cijothomas

…ry#660)

Step 1 - Use new Activity to Replace OT Span

de877fb

reyang reviewed May 4, 2020

View reviewed changes

src/OpenTelemetry/Trace/Configuration/OpenTelemetrySDK.cs Outdated Show resolved Hide resolved

reyang reviewed May 4, 2020

View reviewed changes

pjanotti reviewed May 6, 2020

View reviewed changes

SergeyKanzhelev reviewed May 6, 2020

View reviewed changes

samples/Exporters/Console/TestConsoleActivity.cs Outdated Show resolved Hide resolved

SergeyKanzhelev reviewed May 6, 2020

View reviewed changes

samples/Exporters/Console/TestConsoleActivity.cs Outdated Show resolved Hide resolved

SergeyKanzhelev reviewed May 6, 2020

View reviewed changes

Step 0 - Update to DiagnosticSource 5.0 preview5 from non nuget.org f…

d063b01

…eed.

cijothomas mentioned this pull request May 7, 2020

Update to DiagnosticSource 5.0 preview4 #667

Merged

3 tasks

cijothomas added 4 commits May 7, 2020 00:11

use preview 4 as that contains the change we are after

14b5298

conflict resolv

3ecb6f3

Merge remote-tracking branch 'origin/master' into cithomas/newactivity1

729cda1

update to newer diagnostic source. Adjust to API changes

f61e636

reyang reviewed May 8, 2020

View reviewed changes

src/OpenTelemetry/Trace/Export/SimpleActivityProcessor.cs Outdated Show resolved Hide resolved

reyang reviewed May 8, 2020

View reviewed changes

src/OpenTelemetry/Trace/Export/SimpleActivityProcessor.cs

{

if (!this.disposed)

{

this.disposed = true;

Copy link

Member

reyang May 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not thread safe?

cijothomas added 4 commits May 8, 2020 10:08

minor fixes

9552fa8

use min required version in test

b40be27

few improved example. changed console to print without jsonformat also

8e4b3f9

few small fix

9e452ec

reyang reviewed May 8, 2020

View reviewed changes

Add processing pipeline to Activity

017b289

cijothomas requested review from lmolkova and MikeGoldsmith as code owners May 10, 2020 19:07

comment.

9066f72

reyang approved these changes May 11, 2020

View reviewed changes

CodeBlanch reviewed May 11, 2020

View reviewed changes

src/OpenTelemetry/Trace/Configuration/OpenTelemetryBuilder.cs Outdated Show resolved Hide resolved

bruno-garcia suggested changes May 11, 2020

View reviewed changes

cijothomas added 3 commits May 11, 2020 11:57

reviews coment

3cba75c

rename temp

c988dd1

rename OpenTelemetrySdk

1019ba7

MikeGoldsmith suggested changes May 11, 2020

View reviewed changes

cijothomas mentioned this pull request May 11, 2020

Improvements to SimpleSpanProcessor. #674

Closed

correct header

e895e42

cijothomas mentioned this pull request May 12, 2020

Comparing OpenTelemetry Tracing API with .NET Activity API #675

Closed

CodeBlanch reviewed May 12, 2020

View reviewed changes

pjanotti approved these changes May 12, 2020

View reviewed changes

MikeGoldsmith approved these changes May 13, 2020

View reviewed changes

bruno-garcia approved these changes May 13, 2020

View reviewed changes

MikeGoldsmith merged commit 44b3a43 into open-telemetry:master May 13, 2020

This was referenced May 18, 2020

Support OpenTelemetry concepts on Activity dotnet/runtime#31373

Closed

Distributed Context in ASP.NET and Open Telemetry dotnet/runtime#31372

Closed

This was referenced May 19, 2020

Added sampler to Activity - v1 #683

Merged

Epic - Span API -> Activity API #684

Closed

reyang mentioned this pull request Jun 17, 2020

Add support for multiple pipelines in OpenTelemetryBuilder with Activity #735

Merged

3 tasks

Yun-Ting pushed a commit to Yun-Ting/opentelemetry-dotnet that referenced this pull request Oct 13, 2022

[Exporter.Geneva] Minor refactoring for MsgPackExporter (open-telemet…

79d7498

…ry#660)

		@@ -0,0 +1,76 @@
		// <copyright file="OpenTelemetrySdk1.cs" company="OpenTelemetry Authors">

	// <copyright file="OpenTelemetrySdk1.cs" company="OpenTelemetry Authors">
	// <copyright file="OpenTelemetrySdk.cs" company="OpenTelemetry Authors">

Step 1 - Use new Activity to Replace OT Span #660

Step 1 - Use new Activity to Replace OT Span #660

Conversation

cijothomas commented May 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjanotti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reyang commented May 6, 2020

pjanotti commented May 6, 2020 • edited Loading

cijothomas commented May 6, 2020

cijothomas commented May 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reyang May 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarekgh May 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reyang left a comment

Choose a reason for hiding this comment

CodeBlanch commented May 11, 2020

cijothomas commented May 11, 2020

Choose a reason for hiding this comment

MikeGoldsmith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarekgh May 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarekgh May 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnduhart commented May 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CodeBlanch commented May 12, 2020

SergeyKanzhelev commented May 12, 2020

pjanotti left a comment

Choose a reason for hiding this comment

SergeyKanzhelev commented May 13, 2020

bruno-garcia left a comment

Choose a reason for hiding this comment

cijothomas commented May 4, 2020 •

edited

Loading

pjanotti commented May 6, 2020 •

edited

Loading

reyang May 11, 2020 •

edited

Loading

tarekgh May 11, 2020 •

edited

Loading

tarekgh May 11, 2020 •

edited

Loading

tarekgh May 11, 2020 •

edited

Loading