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.

Sign up for GitHub

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

Jump to bottom

api.propagation.fields should be public #1741

Closed
dyladan opened this issue Dec 10, 2020 · 4 comments · Fixed by #1813
Closed

api.propagation.fields should be public #1741

dyladan opened this issue Dec 10, 2020 · 4 comments · Fixed by #1813
Assignees
@dyladan
Labels
bug Something isn't working

Comments

@dyladan
Copy link
Member

dyladan commented Dec 10, 2020 •
edited
Loading

Besides that the fields method is private method of propagator which you don't have access to - method _getGlobalPropagator is private

This is a bug. You should be able to api.propagation.fields() according to the spec.

originally posted in open-telemetry/opentelemetry-js-contrib#275 (comment)

api.propagation.fields() should return the propagator(s) fields
Make method _getGlobalPropagator from api.propagation public, rename it to getGlobalPropagator
@dyladan dyladan added the bug Something isn't working label Dec 10, 2020
@dyladan dyladan self-assigned this Dec 10, 2020
@dyladan dyladan mentioned this issue Dec 10, 2020
Closed
@obecny
Copy link
Member

obecny commented Dec 10, 2020

it should return all fields from all propagators in case of using CompositePropagator

@dyladan
Copy link
Member Author

dyladan commented Dec 10, 2020

it should return all fields from all propagators in case of using CompositePropagator

That is already the behavior of the composite propagator fields method. The api would just call the current propagator fields, which happens to be the composite.

@blumamir
Copy link
Member

Hi @dyladan ,
I'm wondering if there are any updates on this issue.
in aws-sdk plugin I need to set the propagation fields in the request to sqs, so they will be returned as message attributes from aws which we can then extract context from.
Currently I use hard-coded traceparent and tracestate, but that would fail for propagators other than the HttpTraceContext obviously.
I can create a PR for this issue if you want.

@dyladan
Copy link
Member Author

dyladan commented Jan 13, 2021

I can create a PR for this issue if you want.

Yes please do I would appreciate that.

@blumamir blumamir mentioned this issue Jan 13, 2021
Merged
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this issue Dec 15, 2023
@david-luna
fix: host-metrics system.cpu.utilization calculation fix (open-tele…
b9350d9 
…metry#1741)

* tests: tune the fake clock for testing

* tests: tune the values to expect

* chore: lint autofix

* chore: improve os.cpus mock

* chore: revert implementation for process CPU usage

* chore: rename var

* chore: avoid having useless data upon 1st collection

* chore: remove comments

* chore: change system.cpu attribute names to match semantic conventions

* chore: update test

* chore: lint fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dyladan dyladan

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(api): add public 'fields' function to api.propagator
3 participants
@obecny @dyladan @blumamir