dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.1k stars 4.7k forks source link

Add schema URL support #63651

Open tarekgh opened 2 years ago

tarekgh commented 2 years ago

https://github.com/open-telemetry/opentelemetry-dotnet/issues/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");
ghost commented 2 years ago

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

Issue Details
https://github.com/open-telemetry/opentelemetry-dotnet/issues/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 commented 2 years ago

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

CodeBlanch commented 2 years ago

Just looking at https://github.com/open-telemetry/opentelemetry-dotnet/issues/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 commented 1 year ago

@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 commented 1 year ago

@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

cijothomas commented 1 year ago

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 commented 12 months ago

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 commented 11 months ago

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 commented 6 months ago

OpenTelemetry Semantic Conventions for HTTP Spans are stable.

cijothomas commented 6 months ago

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 commented 5 months ago

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

tarekgh commented 5 months ago

@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 commented 5 months ago

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.