-
Notifications
You must be signed in to change notification settings - Fork 853
Fix ConsoleExporter fails silently when exporting an ActivityLink without Tags.
#3932
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ca14d2f
aafc88a
d8cb758
36a96f0
1e644af
2475c20
c78e501
03928c3
bf0518f
65a6bbb
a8d7f1c
a31481a
f92fe9c
d86e36f
2903d6f
e69e957
c46c86d
a741dbd
f5938cb
4f408c6
e706044
2cb4455
fa1182e
3127932
e65f812
f6ad415
5647a3f
75073f1
464bcee
ddf9cf2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -115,7 +115,7 @@ public override ExportResult Export(in Batch<Activity> batch) | |
| foreach (var activityLink in activity.Links) | ||
| { | ||
| this.WriteLine($" {activityLink.Context.TraceId} {activityLink.Context.SpanId}"); | ||
| foreach (var attribute in activityLink.Tags) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TimothyMothra FYI you could re-write this as: foreach (ref readonly var attribute in activityLink.EnumerateTagObjects())More efficient and it will handle the null case for you 😄 Might be worth re-writing all of the foreach loops in here to use the newer pattern just to have a good reference doing it correctly. Follow-up thing if you are interested 😉
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll take this as a follow up after this PR merges. :) |
||
| foreach (ref readonly var attribute in activityLink.EnumerateTagObjects()) | ||
| { | ||
| if (ConsoleTagTransformer.Instance.TryTransformTag(attribute, out var result)) | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| // <copyright file="ConsoleActivityExporterTest.cs" company="OpenTelemetry Authors"> | ||
| // Copyright The OpenTelemetry Authors | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
| // </copyright> | ||
|
|
||
| using System.Diagnostics; | ||
| using OpenTelemetry.Tests; | ||
| using OpenTelemetry.Trace; | ||
| using Xunit; | ||
|
|
||
| namespace OpenTelemetry.Exporter.Console.Tests; | ||
|
|
||
| public class ConsoleActivityExporterTest | ||
| { | ||
| [Fact] | ||
| public void VerifyConsoleActivityExporterDoesntFailWithoutActivityLinkTags() | ||
| { | ||
| var activitySourceName = Utils.GetCurrentMethodName(); | ||
| using var activitySource = new ActivitySource(activitySourceName); | ||
|
|
||
| var exportedItems = new List<Activity>(); | ||
|
|
||
| using var tracerProvider = Sdk.CreateTracerProviderBuilder() | ||
| .AddSource(activitySourceName) | ||
| .AddInMemoryExporter(exportedItems) | ||
utpilla marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| .Build(); | ||
|
|
||
| ActivityContext context; | ||
| using (var first = activitySource.StartActivity("first")) | ||
| { | ||
| context = first!.Context; | ||
| } | ||
|
|
||
| exportedItems.Clear(); | ||
|
|
||
| var links = new[] { new ActivityLink(context) }; | ||
| using (var secondActivity = activitySource.StartActivity(ActivityKind.Internal, links: links, name: "Second")) | ||
| { | ||
| } | ||
|
|
||
| // Assert that an Activity was exported where ActivityLink.Tags == null. | ||
| var activity = exportedItems[0]; | ||
| Assert.Equal("Second", activity.DisplayName); | ||
| Assert.Null(activity.Links.First().Tags); | ||
|
|
||
| // Test that the ConsoleExporter correctly handles an Activity without Tags. | ||
| using var consoleExporter = new ConsoleActivityExporter(new ConsoleExporterOptions()); | ||
| Assert.Equal(ExportResult.Success, consoleExporter.Export(new Batch<Activity>(new[] { activity }, 1))); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Reword this to something like:
Bug fix to prevent ConsoleExporter from failing when exporting an
ActivityLinkwithout tags.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#3984