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

tracing: allow the Resource to be set externally #4472

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

milas
Copy link
Contributor

@milas milas commented Dec 7, 2023

This is a workaround for the brittleness when constructing OTel Resource objects. Internally, the OTel libraries do their own detection which can be merged with one created in code. However, the semconv spec versions must match. (NOT module version! the semconv package has multiple subpackages for each spec version, e.g. semconv/v1.17, semconv/v1.21, etc.)

This creates a problem when BuildKit is used as a library - the importing app might be using a different, otherwise compatible version of the OTel libraries, so when it creates a resource, it will be merged with one of a different version.

By allowing the Resource to be set (like the Recorder), the calling code can construct a resource using known consistent library versions that work, and then allow BuildKit to take over the rest of the initialization process for OTel.

See also:

@thaJeztah
Copy link
Member

@milas could you fix the DCO failure? Looks like your commit misses a DCO sign-off 🙈

This is a workaround for the brittleness when constructing OTel
`Resource` objects. Internally, the OTel libraries do their own
detection which can be merged with one created in code. However,
the `semconv` spec versions must match. (NOT module version! the
`semconv` package has multiple subpackages for each spec version,
e.g. `semconv/v1.17`, `semconv/v1.21`, etc.)

This creates a problem when BuildKit is used as a library - the
importing app might be using a different, otherwise compatible
version of the OTel libraries, so when it creates a resource, it
will be merged with one of a different version.

By allowing the `Resource` to be set (like the `Recorder`), the
calling code can construct a resource using known consistent
library versions that work, and then allow BuildKit to take over
the rest of the initialization process for OTel.

Signed-off-by: Milas Bowman <milas.bowman@docker.com>
@milas milas marked this pull request as ready for review December 8, 2023 15:55
@milas
Copy link
Contributor Author

milas commented Dec 8, 2023

Oops, fixed and removed from draft. Hopefully CI is happy 🤞🏻

@thaJeztah
Copy link
Member

Looks like it is! I was planning to update my Moby PR to see if this works (temporarily vendoring from your fork 👍)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants