search

signalfx / gdi-specification

Splunk GDI specification for cross-repository compatibility
Apache License 2.0
3 stars 15 forks source link

Deprecate requirement for Jaeger Thrift exporter #190

Closed seemk closed 1 year ago

seemk commented 2 years ago

With the ingest now supporting OTLP over HTTP, it doesn't make much sense to keep Jaeger Thrift exporter as a required exporter.

Jaeger Client repositories themselves are unmaintained, deprecated and a few of the OpenTelemetry libraries still depend on them (e.g. Node.js). This might produce warnings from vulnerability checks without having a fast way to fix the issues.

The proposal is to lower the wording from MUST offer to MAY offer.

Kielek commented 2 years ago

👍 It will reduce amount of work for SplunK OTel .NET AutoInstrumentation. By default we will have otlp over http probably.

FYI: @signalfx/gdi-dotnet-approvers

owais commented 2 years ago

Would be great if it could be dropped but using "MAY" will lead to conconsist UX across Splunk Otel SDKs. I think we should either drop it from all libs or none. We could opt for dropping it from all but allowing users to install it manually as an additional package if they need.

mateuszrzeszutek commented 2 years ago

We could use MAY offer, but add an extra condition that the distro MUST mention that this exporter is deprecated; preferably using a log statement.

owais commented 2 years ago

The scenario I have in mind is someone instrumenting multiple services written in different languages and being surprised that Jaeger works for one but not for the other. They'd eventually find the deprecation statement in docs but still doesn't eliminate the potentially bad UX. Either way, I'll leave it to actual GDI members to decide and go back into my infra cave :)

seemk commented 2 years ago

Agree, ideally I'd strip any mentions to it completely :smile:

seemk commented 2 years ago

But this brings in the question that if we are getting rid of parts of the spec, whether we should have guidelines for distro authors when something should give a deprecation warning and when it's OK to remove something completely.

For example with splunk-otel-js we want to do a 2.0 release soon, it would be great chance to incorporate breaking changes like this.

pellared commented 2 years ago

Personally, I suggest doing it in two phases.

First for GDI spec version 1.x: MAY offer + deprecation notice Second for GDI spec version 2.x: remove it 😉 The instrumentation libraries would have to bump the major version as well.

Side note: As part of spec version 2.x we could also remove SPLUNK_METRICS_* env vars in favor of OTel env vars.

pellared commented 1 year ago

@seemk Do we want to use this issue to track deprecating jeager-thrift-splunk in our distros?

@mzajacsplunk FYI

mateuszrzeszutek commented 1 year ago

@seemk Do we want to use this issue to track deprecating jeager-thrift-splunk in our distros?

We could make 1.5 release of the spec, and then distros will just have to implement it to conform to the new version. And then we just close this issue.

pellared commented 1 year ago

@MrAlias Are the issues with milestone 1.5 required for v1.5.0?

MrAlias commented 1 year ago

@MrAlias Are the issues with milestone 1.5 required for v1.5.0?

They can be bumped to the next release if need be.

pellared commented 1 year ago

I will try to create a new release before end of next week and create issues for spec compliance in directly our repositories (e.g. https://github.com/signalfx/splunk-otel-java). Closing this one.