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 support for OTEL_RESOURCE_DETECTORS environment variable #2948

Open
srikanthccv opened this issue Nov 14, 2022 · 14 comments
Open

Add support for OTEL_RESOURCE_DETECTORS environment variable #2948

srikanthccv opened this issue Nov 14, 2022 · 14 comments
Assignees
Labels
area:configuration Related to configuring the SDK area:sdk Related to the SDK [label deprecated] triaged-rejected [label deprecated] The issue is triaged and rejected by the OTel community spec:resource Related to the specification/resource directory

Comments

@srikanthccv
Copy link
Member

I looked around and didn't find anything related to this. Feel free to point me out if this has been discussed earlier. The SDK default behaviour is to use EnvDetector (using env OTEL_RESOURCE_ATTRIBUTES) and populate some necessary default values. There is no way to configure the SDK to detect and load the resource detector automatically. This is especially important in auto instrumentation cases where the user also wants some custom/third-party resource detector configured via env as with other components such as exporters. I propose adding OTEL_RESOURCE_DETECTORS with default value env and (optionally cloud platform-specific standard detectors). I can send a PR if this is accepted.

The original issue that motivated this open-telemetry/opentelemetry-python-contrib#1372

@srikanthccv srikanthccv added the spec:resource Related to the specification/resource directory label Nov 14, 2022
@arminru arminru added area:configuration Related to configuring the SDK area:sdk Related to the SDK labels Nov 15, 2022
@sanketmehta28
Copy link
Member

Hi @open-telemetry/specs-triagers : any update on this issue?

@reyang
Copy link
Member

reyang commented Nov 18, 2022

FYI #2891 (comment).

@reyang
Copy link
Member

reyang commented Nov 18, 2022

The OTEL_RESOURCE_DETECTORS violates "Anything that has a non-primitive type (see here for the list of primitive types)"

@reyang reyang added the [label deprecated] triaged-rejected [label deprecated] The issue is triaged and rejected by the OTel community label Nov 18, 2022
@srikanthccv
Copy link
Member Author

srikanthccv commented Nov 18, 2022

@reyang, I am not sure I understood it. How is this different from OTEL_TRACES_EXPORTER or any other such variable? How do they not violate the spec, and this does? I am proposing that users configure the name of the resource detector similar to how they would confirm the exporter's name to use.

@reyang
Copy link
Member

reyang commented Nov 18, 2022

It is DETECTOR or DETECTORS?

@srikanthccv
Copy link
Member Author

I am okay with either of the naming but what I am mainly trying to achieve is the SDK to support this capability. An EnvDetector is mandated by spec, which reads the OTEL_RESOURCE_ATTRIBUTES and returns a Resource. There is no way to configure the SDK with additional custom Resources with env. This is a proposal aimed at addressing that gap.

@svrnm
Copy link
Member

svrnm commented Nov 21, 2022

I added a comment to open-telemetry/opentelemetry-python-contrib#1372 already, that OTEL_RESOURCE_DETECTORS might not work as expected:

  • As of now the spec only requires the resource detection from the environment variable OTEL_RESOURCE_ATTRIBUTES being there ootb
  • All other resource detectors are required to be implemented outside of the SDK by the spec
  • This means that as the person running an application (ops) I have no guarantee that the resource detectors I need are available within the application, since the person creating that application (devs) has to add them deliberately, e.g. if OTEL_RESOURCE_DETECTORS=Container,K8s,GCP the Ops person assumes that the devs person has added those 3 resource detectors.
  • So, to make this work there needs to be a coupling between ops & devs to agree on resource detectors available
  • If they do that, why should they not turn them on all by default, since resource detectors should silently fail if they run in an unexpected environment (think going from bare metal to container)

I argue that the way how resource detectors are specified today in the spec, is not practical to end-users: right now the burden to decide which resource detectors are part of an application lays with the developer, although the developer might not have a full understanding on which resources an application will run in the future, e.g. the application might be migrated from one environment to another (let's say from Cloud Vendor A to Cloud Vendor B), or the application might be an open source product used in plenty of environments, like bare metal, docker, different cloud vendors, etc. There are now two options:

  • The developer just includes all of them to make sure that the app runs everywhere.
  • They don't add any resource detector and solely rely on the Ops person putting the right attributes in place via OTEL_RESOURCE_ATTRIBUTES. While this approach makes the ops person responsible, it comes with a bunch of other issues (e.g. you might need to write some shell script to extract the right attributes from somewhere, etc.)

So, the question I am asking myself now, is how can OpenTelemetry provide a practical way for Developers & Operators to detect resources?

@srikanthccv
Copy link
Member Author

All the exporters are also outside of the SDK and implemented as standalone packages. The propagator such as jaeger and b3 are separate and not included in SDK by default but can still be configured with OTEL_PROPAGATORS using comma delimited values like baggage,jaeger,b3. And I am proposing to provide the same capability for the Resources.

@svrnm
Copy link
Member

svrnm commented Nov 22, 2022

All the exporters are also outside of the SDK and implemented as standalone packages. The propagator such as jaeger and b3 are separate and not included in SDK by default but can still be configured with OTEL_PROPAGATORS using comma delimited values like baggage,jaeger,b3. And I am proposing to provide the same capability for the Resources.

Fair point, I had to think about this for a while. From that perspective I agree that having OTEL_RESOURCE_DETECTORS is a meaningful addition to the spec (although I assume the addition is blocked by the moratorium @reyang mentioned here).

Independent of that end-users might need some guidance on which detectors to add in which situation, but that's an issue on it's own.

@joaopgrassi
Copy link
Member

The propagator such as jaeger and b3 are separate and not included in SDK by default

Arent' these included by default in the API though?

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/context/api-propagators.md#propagators-distribution

@srikanthccv
Copy link
Member Author

Arent' these included by default in the API though?

Only tracecontext and baggage are included by default in API. The remaining propagators such jaeger,b3, and ottrace are still provided by OpenTelemetry as extension packages. I am not familiar with all SDKs but this is the case for go, py, java, and js.

@joaopgrassi
Copy link
Member

joaopgrassi commented Nov 22, 2022

Alright :). Remembered I saw the b3 in the API somewhere, but found it: It's in the .NET API package https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/OpenTelemetry.Api/Context/Propagation as has been deprecated so all good.

@srikanthccv
Copy link
Member Author

@reyang, do you still think this should be rejected? I would appreciate the reasoning behind the rejection, given that we have similar capabilities supported for exporters/propagators etc...

@reyang
Copy link
Member

reyang commented Nov 30, 2022

@srikanthccv the reason is captured here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:configuration Related to configuring the SDK area:sdk Related to the SDK [label deprecated] triaged-rejected [label deprecated] The issue is triaged and rejected by the OTel community spec:resource Related to the specification/resource directory
Projects
None yet
Development

No branches or pull requests

7 participants