Skip to content
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.

Sign up for GitHub

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

[Exporter.Console] Add support for ActivitySource.Version #5472

Merged
merged 7 commits into from
Mar 28, 2024
Merged

[Exporter.Console] Add support for ActivitySource.Version #5472

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5d14e22
[Exporter.Console] Add support for ActivitySource.Version
Kielek Mar 25, 2024
0e2aeb0
Merge branch 'main' into exporter.console-activitysource-version
Kielek Mar 26, 2024
a1562ee
Merge branch 'main' into exporter.console-activitysource-version
Kielek Mar 27, 2024
485831b
Merge branch 'main' into exporter.console-activitysource-version
Kielek Mar 27, 2024
5809950
Merge branch 'main' into exporter.console-activitysource-version
Kielek Mar 28, 2024
c385c89
remove redundant line
Kielek Mar 28, 2024
b0d42ca
Merge branch 'main' into exporter.console-activitysource-version
CodeBlanch Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/OpenTelemetry.Exporter.Console/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

* Added support for `ActivitySource.Version` property.
([#5472](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5472))

## 1.8.0-beta.1

Released 2024-Mar-14
Expand Down
5 changes: 5 additions & 0 deletions src/OpenTelemetry.Exporter.Console/ConsoleActivityExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ public override ExportResult Export(in Batch<Activity> batch)
}

this.WriteLine($"Activity.ActivitySourceName: {activity.Source.Name}");
if (!string.IsNullOrEmpty(activity.Source.Version))
{
this.WriteLine($"Activity.ActivitySourceVersion: {activity.Source.Version}");
Copy link
Member

Choose a reason for hiding this comment

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

Why do we want Activity.ActivitySourceVersion instead of Activity.Source.Version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same convention as for ActivitySourceName

Copy link
Member

Choose a reason for hiding this comment

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

It maybe beneficial to use ActivitySourceVersion(aka Tracer version) to make it easier to realize that ActivitySource and Tracer are the same concept.
AND/OR use InstrumentationScope wording?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am less concerned about wording as it is mostly for debugging purposes. It should be fine to have it this value in any form.

I can make additional changes in this PR on follow up PR if we have agreement. It will be great to include this fix on 1.8.0 release so I would like to avoid long discussion.

Copy link
Member

Choose a reason for hiding this comment

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

This is a non-blocking comment. We do have another task to revisit console output format, so any additional improvements can be addressed as part of that.

}

this.WriteLine($"Activity.DisplayName: {activity.DisplayName}");
this.WriteLine($"Activity.Kind: {activity.Kind}");
this.WriteLine($"Activity.StartTime: {activity.StartTimeUtc:yyyy-MM-ddTHH:mm:ss.fffffffZ}");
Expand Down