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

Remove process listener #1522

Merged

Conversation

dyladan
Copy link
Member

@dyladan dyladan commented Sep 11, 2020

Fixes #1521

Replaces #1501

@dyladan dyladan force-pushed the no-process-listener branch from 533e2a3 to a0b3de0 Compare September 11, 2020 20:01
@codecov
Copy link

codecov bot commented Sep 11, 2020

Codecov Report

Merging #1522 into master will decrease coverage by 0.54%.
The diff coverage is 27.27%.

@@            Coverage Diff             @@
##           master    #1522      +/-   ##
==========================================
- Coverage   93.82%   93.28%   -0.55%     
==========================================
  Files         154      153       -1     
  Lines        4761     4750      -11     
  Branches      952      950       -2     
==========================================
- Hits         4467     4431      -36     
- Misses        294      319      +25     
Impacted Files Coverage Δ
packages/opentelemetry-metrics/src/types.ts 100.00% <ø> (ø)
...s/opentelemetry-tracing/src/BasicTracerProvider.ts 77.77% <ø> (-3.62%) ⬇️
packages/opentelemetry-sdk-node/src/sdk.ts 73.91% <20.00%> (-9.14%) ⬇️
...ackages/opentelemetry-metrics/src/MeterProvider.ts 88.23% <100.00%> (-2.25%) ⬇️
...ges/opentelemetry-metrics/src/export/Controller.ts 45.45% <0.00%> (-31.82%) ⬇️
...ges/opentelemetry-tracing/src/NoopSpanProcessor.ts 42.85% <0.00%> (-28.58%) ⬇️
...s/opentelemetry-metrics/src/export/NoopExporter.ts 25.00% <0.00%> (-25.00%) ⬇️
...elemetry-tracing/src/export/ConsoleSpanExporter.ts 78.57% <0.00%> (-21.43%) ⬇️
packages/opentelemetry-metrics/src/Meter.ts 87.80% <0.00%> (-6.51%) ⬇️

@Flarna
Copy link
Member

Flarna commented Sep 12, 2020

Is this just a revert of #1321 or are there more changes?
If yes, should we reopen #1290 which was closed by #1321?

Copy link
Member

@obecny obecny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm,
I would just add an example (in getting started for example or more appropriate place) to show people that if they want to mimic the removed behaviour in node they should simply listen for sigterm and call shutdown of tracer etc..

@dyladan
Copy link
Member Author

dyladan commented Sep 21, 2020

lgtm,
I would just add an example (in getting started for example or more appropriate place) to show people that if they want to mimic the removed behaviour in node they should simply listen for sigterm and call shutdown of tracer etc..

f0cddd0 (#1522)

@dyladan dyladan merged commit 5489f96 into open-telemetry:master Sep 21, 2020
@dyladan dyladan deleted the no-process-listener branch September 21, 2020 19:28
Flarna added a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Nov 19, 2020
Graceful shutdown support was removed via
open-telemetry#1522

Remove the corresponding configuration because it is not used anymore.

Refs: open-telemetry#1321
dyladan pushed a commit that referenced this pull request Nov 25, 2020
Graceful shutdown support was removed via
#1522

Remove the corresponding configuration because it is not used anymore.

Refs: #1321
SuperButterfly pushed a commit to SuperButterfly/opentelemetry-js that referenced this pull request Sep 18, 2022
Graceful shutdown support was removed via
open-telemetry/opentelemetry-js#1522

Remove the corresponding configuration because it is not used anymore.

Refs: open-telemetry/opentelemetry-js#1321
viridius-1 pushed a commit to viridius-1/opentelemetry-js that referenced this pull request Jan 14, 2023
Graceful shutdown support was removed via
open-telemetry/opentelemetry-js#1522

Remove the corresponding configuration because it is not used anymore.

Refs: open-telemetry/opentelemetry-js#1321
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.

Consider removing the SIGTERM handler
5 participants