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

The SDK MUST extract information from the OTEL_RESOURCE_ATTRIBUTES environment variable #1689

Closed
MrAlias opened this issue Mar 10, 2021 · 4 comments · Fixed by #1785 · May be fixed by open-o11y/opentelemetry-go#69
Closed
Assignees
Labels
area:resources Part of OpenTelemetry resources good first issue Good for newcomers pkg:SDK Related to an SDK package
Milestone

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Mar 10, 2021

Currently we do not default to including attributes specified with the OTEL_RESOURCE_ATTRIBUTES environment variable. This is required by the specification:

The SDK MUST extract information from the OTEL_RESOURCE_ATTRIBUTES environment variable and merge this, as the secondary resource, with any resource information provided by the user, i.e. the user provided resource information has higher priority.

The OTEL_RESOURCE_ATTRIBUTES environment variable will contain of a list of key value pairs, and these are expected to be represented in a format matching to the W3C Baggage, except that additional semi-colon delimited metadata is not supported, i.e.: key1=value1,key2=value2. All attribute values MUST be considered strings.

These attributes will be used to create a Resource if resource.New is used to create a resource passed to an SDK, but it must be done by the user.

The TracerProvider and metric Controller need to be updated to default extracting these attributes into a resource that is merged with the resource.Default resource they already use.

Having these attributes automatically merged into the used resource makes the FromEnv detector of limited value. It would only include attributes that would latter be merged in. It seems like that detector should be removed, or possibly changed to accept a list of environment variable names that it would then pull in as attributes of a Resource. Meaning if you wanted to specify a Resource in a way other than defined for the OTEL_RESOURCE_ATTRIBUTES environment variable you could accomplish it that way.

@MrAlias MrAlias added pkg:SDK Related to an SDK package release:required-for-ga area:resources Part of OpenTelemetry resources labels Mar 10, 2021
@MrAlias MrAlias added this to the RC1 milestone Mar 10, 2021
@evantorrie
Copy link
Contributor

I think this will also apply to Resource.Default(), i.e. if OTEL_RESOURCE_ATTRIBUTES is defined in the environment, then Resource.Default() should include those (to the point of overriding say the default determined service.name).

@MrAlias
Copy link
Contributor Author

MrAlias commented Mar 11, 2021

I think this will also apply to Resource.Default(), i.e. if OTEL_RESOURCE_ATTRIBUTES is defined in the environment, then Resource.Default() should include those (to the point of overriding say the default determined service.name).

From what I can tell the environment resource detector is not included in the default:

}(Detect(context.Background(), defaultServiceNameDetector{}, TelemetrySDK{}))

But, there would still be a problem if it was that the user supplied resource to an SDK would not be merged with the environment attributes. Instead it would overwrite or clear them:

c.Resource = cfg.Resource
if c.Resource == nil {
c.Resource = resource.Default()
}

@cynthiakedu
Copy link
Contributor

@Aneurysm9 Can I take this issue?

@cynthiakedu cynthiakedu removed their assignment Mar 26, 2021
@dhruv-vora
Copy link
Contributor

@Aneurysm9 I'd like to take this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:resources Part of OpenTelemetry resources good first issue Good for newcomers pkg:SDK Related to an SDK package
Projects
None yet
5 participants