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.

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

Support Telemetry Schema Url in tracing and metrics #113034

Merged
merged 4 commits into from
Mar 4, 2025

Conversation

tarekgh
Copy link
Member

@tarekgh tarekgh commented Mar 1, 2025

Fixes #63651

@Copilot Copilot bot review requested due to automatic review settings March 1, 2025 19:08
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@tarekgh tarekgh added this to the 10.0.0 milestone Mar 1, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR adds support for a Telemetry Schema URL parameter in tracing and metrics to address #63651. The changes include introducing a TelemetrySchemaUrl property in ActivitySourceOptions, Meter, and related types; updating constructor and initialization signatures to propagate the new parameter; and adjusting MetricsEventSource event definitions and tests to include the TelemetrySchemaUrl.

Reviewed Changes

File Description
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/ActivitySourceOptions.cs Added TelemetrySchemaUrl property to options used for ActivitySource creation
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs Updated initialization and constructors to include telemetrySchemaUrl parameter
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/ActivitySource.cs Updated ActivitySource constructors and properties to propagate TelemetrySchemaUrl
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MeterOptions.cs Added TelemetrySchemaUrl property to MeterOptions
src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricEventSourceTests.cs Updated Meter creation in tests to supply the new TelemetrySchemaUrl value
src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs Added TelemetrySchemaUrl assertions in ActivitySource tests
src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs Updated reference definitions to expose TelemetrySchemaUrl property and overloads
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs Updated event methods to include TelemetrySchemaUrl in event logging
src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs Updated meter creation tests to check TelemetrySchemaUrl handling

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

@tarekgh
Copy link
Member Author

tarekgh commented Mar 1, 2025

CC @noahfalk @lmolkova @samsp-msft @CodeBlanch @rajkumar-rangaraj

@tarekgh
Copy link
Member Author

tarekgh commented Mar 1, 2025

/azp run runtime-libraries-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tarekgh
Copy link
Member Author

tarekgh commented Mar 2, 2025

/azp run runtime-libraries-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@CodeBlanch CodeBlanch left a comment

Choose a reason for hiding this comment

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

LGTM

@samsp-msft
Copy link
Member

So what is required for this to work end to end? Is more needed from the OTel SDK and the OTLP exporter?

@tarekgh
Copy link
Member Author

tarekgh commented Mar 3, 2025

So what is required for this to work end to end? Is more needed from the OTel SDK and the OTLP exporter?

Yes. @lmolkova is following up and planning to use it in E2E scenario.

@CodeBlanch
Copy link
Contributor

So what is required for this to work end to end? Is more needed from the OTel SDK and the OTLP exporter?

Ya OTel SDK will need to be updated to set these new SchemaUrl properties on the instrumentation scope when serializing. For example here is where we handle ActivitySource Version + Tags: https://github.com/open-telemetry/opentelemetry-dotnet/blob/1b555c1201413f2f55f2cd3c4ba03ef4b615b6b5/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/Serializer/ProtobufOtlpTraceSerializer.cs#L127-L132

Copy link

@rajkumar-rangaraj rajkumar-rangaraj left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@Kielek Kielek left a comment

Choose a reason for hiding this comment

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

🎉 Great to see this!

@tarekgh tarekgh merged commit 9d16e26 into dotnet:main Mar 4, 2025
80 of 91 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add schema URL support
6 participants