Skip to content
This repository has been archived by the owner on Nov 10, 2022. It is now read-only.

[spec] Do not export NOOP implementations #50

Closed
dyladan opened this issue Apr 27, 2021 · 4 comments
Closed

[spec] Do not export NOOP implementations #50

dyladan opened this issue Apr 27, 2021 · 4 comments
Assignees
@dyladan
Labels
specification

Comments

@dyladan
Copy link
Member

dyladan commented Apr 27, 2021 •
edited
Loading

The NOOP implementations are currently exported. Particularly the NOOP tracer is used to create NOOP spans. After #51, this will no longer be required. Because the specification does not say to export NOOP implementations, we should remove their exports before 1.0
@dyladan dyladan self-assigned this Apr 27, 2021
@dyladan dyladan mentioned this issue Apr 27, 2021
Merged
dyladan added a commit to dyladan/opentelemetry-js-api that referenced this issue Apr 27, 2021
@dyladan
feat: add function to wrap SpanContext in NonRecordingSpan open-telem…
767028c 
…etry#50
@dyladan dyladan mentioned this issue Apr 27, 2021
Merged
@Flarna
Copy link
Member

Flarna commented Apr 27, 2021 •
edited
Loading

As far as I know this is already the case.

Refs: open-telemetry/opentelemetry-js#1899 (that time it was still named NoopSpan)

@dyladan
Copy link
Member Author

dyladan commented Apr 27, 2021 •
edited
Loading

We do export NonRecordingSpan. The spec requires us not to. After #51 it will be possible to create one using that method so we will no longer have to export the constructor directly.

edit: it seems you were right we do not. but we do export the noop tracer which the API does not say to export. IMO we should limit exports that are not spec-required if at all possible.

@dyladan dyladan changed the title [spec] Do not export NonRecordingSpan [spec] Do not export NOOP implementations Apr 27, 2021
@dyladan
Copy link
Member Author

dyladan commented Apr 27, 2021

@Flarna updated description

@dyladan dyladan linked a pull request May 5, 2021 that will close this issue
chore: do not export singletons #46
Merged
@dyladan dyladan removed a link to a pull request May 5, 2021
chore: do not export singletons #46
Merged
@dyladan
Copy link
Member Author

dyladan commented Jun 2, 2021

Done in #46

@dyladan dyladan closed this as completed Jun 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@dyladan dyladan

Labels
specification
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dyladan @Flarna