feat: return errors from resource.New #59
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which problem is this PR solving?
When using detectors with mis-matched Schema URLs, resource.New will return an error. Previously, this was dropped and consequently it looked like configuration worked but many resource attributes would be missing in the resulting telemetry.
With the recent addition of #48, this error is much easier to hit. For example, the detectors built into opentelemetry-go import semconv
v1.17.0
but this project pulls inv1.18.0
. This means that adding something likeotelconfig.WithResourceOption(resource.WithHost())
results in a mis-configured OTel and no error to warn you what happened.This is all … very bad. This PR is really the bare minimum that needs to be done here. That is, it at least causes a runtime error so that you can tell what's going on — but it doesn't solve the problem that you fundamentally cannot use this library with any other library that doesn't happen to use semconv
v1.18.0
. There are a number of open issues in OTel and adjacent repositories regarding this problem, which probably warrant further investigation into a long-term sustainable solution.Short description of the changes
Return the
err
fromresource.New
up through the stack. Update tests to expect this.How to verify that this has the expected result
There's a new test that specifically uses a detector with a different schema URL.