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

Add Graceful Shutdown Support #1321

Merged
merged 69 commits into from
Aug 17, 2020
Merged

Add Graceful Shutdown Support #1321

Changes from 1 commit
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
871d696
feat: graceful shutdown for tracing and metrics
jonahrosenblum Jul 16, 2020
da633ad
Merge remote-tracking branch 'upstream/master' into master
jonahrosenblum Jul 16, 2020
de161cc
fix: wording in test case
jonahrosenblum Jul 16, 2020
edd3b49
fix: typo
jonahrosenblum Jul 16, 2020
16c0faf
fix meterprovider config to use bracket notation
jonahrosenblum Jul 16, 2020
f0b9ef6
fix meterprovider config to use bracket notation
jonahrosenblum Jul 16, 2020
4bd85b5
fix: add callbacks to shutdown methods
jonahrosenblum Jul 17, 2020
86cc4e9
fix: merge conflict
jonahrosenblum Jul 17, 2020
5d93f6f
fix: merge conflict
jonahrosenblum Jul 17, 2020
9ad87e5
simplify meter shutdown code
jonahrosenblum Jul 17, 2020
1d3ea53
Merge remote-tracking branch 'upstream/master' into master
jonahrosenblum Jul 17, 2020
0a99520
fix: fix one-liner
jonahrosenblum Jul 17, 2020
791156b
private function name style fix
jonahrosenblum Jul 18, 2020
7d714b6
fix: naming of private member variables
jonahrosenblum Jul 18, 2020
b106b50
fix: graceful shutdown now works in browser
jonahrosenblum Jul 22, 2020
ba5be87
Merge remote-tracking branch 'upstream/master' into master
jonahrosenblum Jul 22, 2020
064c8e8
fix: window event listener will trigger once
jonahrosenblum Jul 22, 2020
54a82c5
fix: modify global shutdown helper functions
jonahrosenblum Jul 23, 2020
8fd53c8
fix: remove callback from remove listener args
jonahrosenblum Jul 23, 2020
b5af380
fix: change global shutdown function names and simplify functionality
jonahrosenblum Jul 23, 2020
e159546
fix: add rest of function refactoring and simplification
jonahrosenblum Jul 23, 2020
c0d3f5f
fix: remove unintended code snippet
jonahrosenblum Jul 23, 2020
9ceba0e
fix: refactor naming of listener cleanup function and fix sandbox issue
jonahrosenblum Jul 24, 2020
078802d
fix: make global shutdown cleanup local
jonahrosenblum Jul 24, 2020
adedac1
Merge remote-tracking branch 'upstream/master' into master
jonahrosenblum Jul 24, 2020
44a7275
fix: change interval of MeterProvider collection to ensure it does no…
jonahrosenblum Aug 3, 2020
a1bfb12
fix: resolve merge conflict
jonahrosenblum Aug 3, 2020
3dd51d2
Merge remote-tracking branch 'upstream/master' into master
jonahrosenblum Aug 3, 2020
d541837
chore: removing _cleanupGlobalShutdownListeners
obecny Aug 3, 2020
0163241
fix: remove unnecesary trace provider member function
jonahrosenblum Aug 4, 2020
95bb8bf
Merge remote-tracking branch 'upstream/master' into master
jonahrosenblum Aug 4, 2020
ef84611
Merge branch 'master' of /~https://github.com/open-telemetry/openteleme…
jonahrosenblum Aug 5, 2020
6f4de1e
Removing default span attributes (#1342)
aravinsiva Jul 27, 2020
5597957
feat: add baggage support to the opentracing shim (#918)
rubenvp8510 Jul 27, 2020
7c09806
Add nodejs sdk package (#1187)
dyladan Jul 27, 2020
b956e9c
feat: add OTEL_LOG_LEVEL env var (#974)
Jul 27, 2020
ab0de32
Proto update to latest to support arrays and maps (#1339)
obecny Jul 27, 2020
c4ffad7
chore: 0.10.0 release proposal (#1345)
dyladan Jul 27, 2020
32db1a9
fix: add missing grpc-js index (#1358)
dyladan Jul 28, 2020
db4745e
chore: 0.10.1 release proposal (#1359)
dyladan Jul 28, 2020
0a0a487
feat(api/context-base): change compile target to es5 (#1368)
markwolff Jul 31, 2020
802c5ad
Feat: Make ID generator configurable (#1331)
EdZou Jul 31, 2020
e6d49b4
fix: require grpc-js instead of grpc in grpc-js example (#1364)
reggiemcdonald Jul 31, 2020
5727e6e
chore(deps): update all non-major dependencies (#1371)
renovate-bot Jul 31, 2020
451c99c
chore: bump metapackage dependencies (#1383)
dyladan Aug 3, 2020
9336bfa
chore: 0.10.2 proposal (#1382)
dyladan Aug 3, 2020
bf46c1b
fix: remove unnecesary trace provider member function
jonahrosenblum Aug 4, 2020
62966b5
refactor(metrics): distinguish different aggregator types (#1325)
legendecas Aug 4, 2020
d07163a
Propagate b3 parentspanid and debug flag (#1346)
srjames90 Aug 4, 2020
f0323e6
feat: Export MinMaxLastSumCountAggregator metrics to the collector as…
davidwitten Aug 4, 2020
4d92d39
feat: Collector Metric Exporter for the Web (#1308)
davidwitten Aug 5, 2020
70bee58
Fix issues in TypeScript getting started example code (#1374)
mickdekkers Aug 5, 2020
ef046b9
chore: deploy canary releases (#1384)
dyladan Aug 6, 2020
5b14fb5
fix: addressing failed merge
jonahrosenblum Aug 7, 2020
73edc9a
Merge branch 'master' of /~https://github.com/jonahrosenblum/openteleme…
jonahrosenblum Aug 7, 2020
12b18fd
fix: protos pull
jonahrosenblum Aug 7, 2020
b0fdffd
fix: address marius' feedback
jonahrosenblum Aug 11, 2020
81f7dee
chore: merge with pr_1321
obecny Aug 11, 2020
93719b9
chore: deleting removeAllListeners from prometheus, fixing tests, cle…
obecny Aug 11, 2020
a0d309d
Merge branch 'master' into pr_1321_fix
obecny Aug 11, 2020
a28cfb0
Merge branch 'master' into master
obecny Aug 11, 2020
9c21dbb
Merge pull request #2 from obecny/pr_1321_fix
jonahrosenblum Aug 11, 2020
bfa1a0a
fix: add documentation and cleanup code
jonahrosenblum Aug 12, 2020
ec3ce12
fix: remove async label from shutdown and cleanup test case
jonahrosenblum Aug 12, 2020
49e1a69
fix: update controller collect to return promise
jonahrosenblum Aug 12, 2020
8afa9a1
fix: make downsides of disabling graceful shutdown more apparent
jonahrosenblum Aug 15, 2020
32439ab
Merge branch 'master' into master
jonahrosenblum Aug 17, 2020
529df9b
Merge branch 'master' into master
dyladan Aug 17, 2020
d5c430c
Merge branch 'master' into master
obecny Aug 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: merge conflict
  • Loading branch information
@jonahrosenblum
jonahrosenblum committed Jul 17, 2020
commit 5d93f6fae818d7bb922ae931e6a74cc4bed0f1d5
6 changes: 0 additions & 6 deletions packages/opentelemetry-metrics/src/MeterProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ export class MeterProvider implements api.MeterProvider {
return this._meters.get(key)!;
}

<<<<<<< HEAD
shutdown(cb: () => void = () => {}) {
jonahrosenblum marked this conversation as resolved.
Show resolved Hide resolved
this.onShutdown().then(() => {
setTimeout(cb, 0);
Expand All @@ -68,11 +67,6 @@ export class MeterProvider implements api.MeterProvider {
private async onShutdown() {
dyladan marked this conversation as resolved.
Show resolved Hide resolved
if (this._config.exporter) {
this._config.exporter.shutdown();
Copy link

Choose a reason for hiding this comment

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

Is this._config.exporter.shutdown() actually blocking, or is this a missing callback or missing async decoration?
It may be out of scope to fix, but an issue could be created to look into this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think exporter shutdown calls are async. Regardless I'm not sure if it matters if this is blocking?

Copy link
Member

Choose a reason for hiding this comment

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

I think the exporters don't do any batching so it should be ok to have the shutdown be sync. On the other hand, it may be worth adding to the API just to allow the flexibility in the future.

Copy link

Choose a reason for hiding this comment

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

Technically, the exporters could batch here: Say you have 1K unflushed spans, and the backend API supports 100 spans per batch call with a 500 ms limit between calls. -> shutdown would take 5 seconds.

I'm not sure it matters either. Perhaps something to potentially address in a future API design sprint.

=======
shutdown(): void {
if (this._config.exporter) {
this._config['exporter'].shutdown();
>>>>>>> f0b9ef652d7e938ffbce100ddf9a8eb30840961d
}
const shutdownPromises: Promise<void>[] = [];
this._meters.forEach((meter, _) => {
Expand Down