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

Add schema URL support #63651

Open
tarekgh opened this issue Jan 11, 2022 · 13 comments
Open

Add schema URL support #63651

tarekgh opened this issue Jan 11, 2022 · 13 comments
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Diagnostics.Activity feature-request
Milestone

Comments

@tarekgh
Copy link
Member

tarekgh commented Jan 11, 2022

open-telemetry/opentelemetry-dotnet#2417
https://github.com/open-telemetry/opentelemetry-specification/pull/1666/files

Telemetry sources such as instrumented applications and consumers of telemetry such as observability backends sometimes make implicit assumptions about the emitted telemetry. They assume that the telemetry will contain certain attributes or otherwise have a certain shape and composition of data. Essentially there is a coupling between 3 parties: OpenTelemetry semantic conventions, telemetry sources and telemetry consumers. The coupling complicates the independent evolution of these 3 parties. The 3 parties should be able to evolve independently over time and Telemetry Schemas are central to how we make this possible.

Proposal

namespace System.Diagnostics
{
    public sealed class ActivitySource : IDisposable
    {
        public ActivitySource(string name, string? version = "") { ... }
+       public ActivitySource(string name, string? version, string? telemetrySchemaUrl) { ... }
+       public string? TelemetrySchemaUrl { get; }
    }
}

namespace System.Diagnostics.Metrics
{
    public class Meter : IDisposable
    {
        public Meter(string name) { }
        public Meter(string name, string? version)  { }
+       public Meter(string name, string? version, string? telemetrySchemaUrl)  { }
+       public string? TelemetrySchemaUrl { get; }
    }
}

Usage

    private static ActivitySource s_source = new ActivitySource("MyLibrary.MyFeature", "2.0", "https://opentelemetry.io/schemas/1.2.0");
    private static Meter s_meter = new Meter("MyLibrary.MyFeature", "2.0", "https://opentelemetry.io/schemas/1.2.0");
@tarekgh tarekgh added this to the 7.0.0 milestone Jan 11, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jan 11, 2022
@ghost
Copy link

ghost commented Jan 11, 2022

Tagging subscribers to this area: @dotnet/area-system-diagnostics-activity
See info in area-owners.md if you want to be subscribed.

Issue Details

open-telemetry/opentelemetry-dotnet#2417
https://github.com/open-telemetry/opentelemetry-specification/pull/1666/files

Author: tarekgh
Assignees: -
Labels:

feature request, area-System.Diagnostics.Activity

Milestone: 7.0.0

@tarekgh tarekgh removed the untriaged New issue has not been triaged by the area owner label Jan 11, 2022
@tarekgh tarekgh added the api-needs-work API needs work before it is approved, it is NOT ready for implementation label Feb 4, 2022
@tarekgh
Copy link
Member Author

tarekgh commented Feb 7, 2022

We are going to wait for finalizing the semantic convention discussion on OpenTelemetry side to know if there will be any effect on this current proposal. We expect to have the feedback by the end of March or early April.

CC @cijothomas @reyang @denisivan0v

@tarekgh tarekgh modified the milestones: 7.0.0, Future Apr 14, 2022
@CodeBlanch
Copy link
Contributor

Just looking at open-telemetry/opentelemetry-dotnet#3445. There are cases where instrumentation does not own the ActivitySource being used (ex AspNetCore, HttpClient). In those cases it might be helpful to make some of this data settable?

namespace System.Diagnostics
{
    public sealed class ActivitySource : IDisposable
    {
        public ActivitySource(string name, string? version = "") { ... }
+       public ActivitySource(string name, string? version, string? telemetrySchemaUrl) { ... }
        public string? Version { 
          get;
+         set;
        }
+       public string? TelemetrySchemaUrl { get; set; }
    }
}

That would allow (for example) OpenTelemetry instrumentation augmenting the sources to pass along its version + schema url for the schema being augmented.

Breaks if users have multiple augmenting libraries/schemas, but may not be a concern?

@danelson
Copy link

danelson commented Sep 5, 2023

@tarekgh I am curious if this was revisited at all with some of the changes made for .NET 8 (e.g. https://github.com/lmolkova/semantic-conventions/blob/dotnet8-metrics/docs/dotnet/dotnet-http-metrics.md). There was a comment that mentioned staying compatible with OTel but I am a bit concerned that this will be hard for consumers to manage if there are additional changes without the existence of schema URL. Do you have any idea when I might be able to expect this change?

Speaking from someone whose organization sticks to mainly LTS releases, if this does not make .NET 8 then it becomes hard to manage across various language SDKs.

@tarekgh
Copy link
Member Author

tarekgh commented Sep 5, 2023

@danelson the semantic convention discussion came very late in the 8.0 after passing the point of adding a new API. This is something we can consider for our next release.

CC @reyang @noahfalk @CodeBlanch

@tarekgh tarekgh modified the milestones: Future, 9.0.0 Sep 5, 2023
@cijothomas
Copy link
Contributor

if this does not make .NET 8 then it becomes hard to manage across various language SDKs.

I doubt if this is going to be part of .NET 8 as it is too late. The earliest, hence, would be .NET 9. And I don' think it is relevant that your org recommends LTS only, as this change will be part of DiagnosticSource package, which is out-of-band, and is not tired to the actual .NET runtime.

I believe the main reason why OTel .NET has not pushed for this to land in .NET 8 was that, the use cases of the schema url are still an experimental spec:
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/telemetry-stability.md

(the schema_url part of tracer/meter itself is stable long ago, the above is just the spec about the schemas/ and its transformations etc.)

@pellared
Copy link

pellared commented Oct 13, 2023

Does it mean that we won't get any stable OpenTelemetry .NET instrumentation library before .NET 9 release?

Take notice that the schema URL support is required by OpenTelemetry specification since 2021-06-07 (sic!).

@cijothomas
Copy link
Contributor

Does it mean that we won't get any stable OpenTelemetry .NET instrumentation library before .NET 9 release?

I don't think so. As the instrumentation libraries are expected to move to stable in couple months from what i heard (though there are lot of pending issues)

The spec which talks about schema_url usages (transformation etc.)m fixed telemetry schema etc. are still experimental, with no sign of moving to stable. Even if they are needed, it should be possible to not change any schema OR wait until schema_url is available before making any changes. The schema_url could come from .NET itself OR we might leverage Meter attributes.

@pellared
Copy link

@cijothomas
Copy link
Contributor

OpenTelemetry Semantic Conventions for HTTP Spans are stable.

But that is not relevant to this issue right?

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/telemetry-stability.md is still experimental, though that does not have to prevent us from adding schema_url support.

@lmolkova
Copy link

lmolkova commented May 1, 2024

@tarekgh is it in scope for .NET 9? thanks!

@tarekgh
Copy link
Member Author

tarekgh commented May 1, 2024

@lmolkova no this is not in 9.0.

The fixed schema is the only stable part of the specs which doesn't require URL.

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/telemetry-stability.md#fixed-schema-telemetry-producers.

CC @reyang

@lmolkova
Copy link

lmolkova commented May 2, 2024

So none of the .NET instrumentation can change after they are declared stable until at least .NET 10? Sounds like we're going to break this rule a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Diagnostics.Activity feature-request
Projects
None yet
Development

No branches or pull requests

6 participants