feat: Opentelemetry integration #1078
Conversation
* changes without context
autosynth cannot find the source of changes triggered by earlier changes in this
repository, or by version upgrades to tools such as linters.
* fix: rename _toc to toc
Source-Author: F. Hinkelmann <franziska.hinkelmann@gmail.com>
Source-Date: Tue Jul 21 10:53:20 2020 -0400
Source-Repo: googleapis/synthtool
Source-Sha: 99c93fe09f8c1dca09dfc0301c8668e3a70dd796
Source-Link: googleapis/synthtool@99c93fe
Co-authored-by: sofisl <55454395+sofisl@users.noreply.github.com>
Source-Author: F. Hinkelmann <franziska.hinkelmann@gmail.com> Source-Date: Thu Jul 23 01:45:04 2020 -0400 Source-Repo: googleapis/synthtool Source-Sha: 3a00b7fea8c4c83eaff8eb207f530a2e3e8e1de3 Source-Link: googleapis/synthtool@3a00b7f
…into opentelemetry-integration
google-cla
bot
added
the
cla: yes
There was a problem hiding this comment.
I left a few notes/nits.
My main feedback is that I would start by making the open telemetry feature optional. If you made it configurable with an environment variable, we could start by having select customers enable this environment variable places like GCF/Cloud Run.
I want to be very mindful of customers who are running PubSub out of GCP environments, e.g., other clouds.
|
Thanks for doing this @sethmaxwl and also thanks for the review @bcoe! I agree with what Ben's saying, re: what I wrote above, that we want this to basically be invisible for existing users until they opt in. It sounds like the dependencies are okay, we just want to avoid having it do anything new without an opt-in. The CI issues look like they're mostly about an ongoing TypeScript version issue. I'm working on a fix for that so we can get several PRs back on track. |
Codecov Report
@@ Coverage Diff @@
## master #1078 +/- ##
==========================================
+ Coverage 95.06% 97.78% +2.71%
==========================================
Files 24 25 +1
Lines 10925 11067 +142
Branches 483 546 +63
==========================================
+ Hits 10386 10822 +436
+ Misses 535 241 -294
Partials 4 4
Continue to review full report at Codecov.
|
bcoe
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
sethmaxwl
force-pushed
the
opentelemetry-integration
branch
from
15affee
to
0606f83
Compare
…xwl/nodejs-pubsub into opentelemetry-integration
…xwl/nodejs-pubsub into opentelemetry-integration
| @@ -30,7 +30,7 @@ | |||
| "presystem-test": "npm run compile", | |||
| "system-test": "mocha build/system-test --timeout 600000", | |||
| "samples-test": "cd samples/ && npm link ../ && npm install && npm test && cd ../", | |||
| "test": "c8 mocha build/test", | |||
| "test": "c8 mocha build/test --recursive", | |||
There was a problem hiding this comment.
We've got a change pending actually that will obsolete this, but I will deal with it when that gets merged.
There was a problem hiding this comment.
I just added it here to get CI to run the publisher tests too.
| }, SUBSCRIBER_TIMEOUT * 1000); | ||
| } | ||
|
|
||
| publishMessage().then(subscriptionListen()); |
There was a problem hiding this comment.
This is missing a doc end tag, though it may not matter at this point if nothing in the docsite is expecting it to be here. (We would need to canonicalize the the tag name anyway.)
There was a problem hiding this comment.
I want to include a link to this example in the blog post. How should we canonicalize the tag name?
There was a problem hiding this comment.
Let me see if I can find the right person to ask... there were some changes in that area lately.
feywind
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
feywind
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
Adds OpenTelemetry tracing to messages in order to provide more visibility into how messages are behaving behind the scenes. @opentelemetry/api and @opentelemetry/tracing are added as dependencies.
Fixes #1061