-
Notifications
You must be signed in to change notification settings - Fork 806
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
fix(resources): extend ResourceAttributes interface to comply with spec #2924
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2924 +/- ##
==========================================
- Coverage 93.04% 93.03% -0.02%
==========================================
Files 183 183
Lines 6055 6055
Branches 1297 1297
==========================================
- Hits 5634 5633 -1
- Misses 421 422 +1
|
I thought that this fix is not a breaking change, as we only broader the interface options.
I suspect that this change broke the compatibility between 1.2.0 and 1.3.0, as the interfaces are now different. |
Looks like you have two instances of Maybe you pinned some older dependency? |
For reference: the issue is discussed under open-telemetry/opentelemetry-js-contrib#1042 |
There could be multiple ways to address this fix. I choose to use the interfaces from
@opentelemetry/api
but am open to other suggestions.Which problem is this PR solving?
Fixes #2907
The current interface of
ResourceAttributes
supports only a subset of what is defined in the spec.An array of primitive values are allowed in spec but are missing in the interface declaration in
@opentelemetry/resources
Short description of the changes
Use the same type from
@opentelemetry/api
.We should have used
Attributes
interface but since it's introduced in1.1.0
I choose theSpanAttributes
interface which will allow the SDK to work correctly with versions 1.0.x of the api.This change only broader the definition of the
ResourceAttributes
interface so it is not expected to break anything.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
This is only change in typescript interface. Tested via CI run of current build / test / lint actions.
Checklist: