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

add ioredis instrumentation plugin #410

Merged
merged 1 commit into from Mar 14, 2019

Conversation

Projects
None yet
5 participants
@vmarchaud
Copy link
Contributor

vmarchaud commented Mar 10, 2019

@googlebot googlebot added the cla: yes label Mar 10, 2019

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Mar 10, 2019

Codecov Report

Merging #410 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #410      +/-   ##
=========================================
- Coverage   94.81%   94.8%   -0.01%     
=========================================
  Files         136     138       +2     
  Lines        9000    9086      +86     
  Branches      666     667       +1     
=========================================
+ Hits         8533    8614      +81     
- Misses        467     472       +5
Impacted Files Coverage Δ
src/prometheus-stats.ts 92.78% <0%> (-1.1%) ⬇️
src/stats/view.ts 97.43% <0%> (-0.24%) ⬇️
test/test-view.ts 98.17% <0%> (-0.21%) ⬇️
src/stats/recorder.ts 97.56% <0%> (-0.06%) ⬇️
src/stackdriver-cloudtrace-utils.ts 97.26% <0%> (-0.04%) ⬇️
src/stats/types.ts 100% <0%> (ø) ⬆️
src/http.ts 91.21% <0%> (ø) ⬆️
test/test-recorder.ts 100% <0%> (ø) ⬆️
test/test-prometheus-stats.ts 100% <0%> (ø) ⬆️
src/mongodb.ts 91.78% <0%> (ø) ⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3111ab8...b2b9a13. Read the comment docs.

@draffensperger
Copy link
Contributor

draffensperger left a comment

Thanks for the contribution and for use of strong types where possible!

@vmarchaud vmarchaud force-pushed the vmarchaud:ioredis-plugin branch from 04c1cb0 to aca471f Mar 13, 2019

@vmarchaud vmarchaud requested a review from justindsmith as a code owner Mar 13, 2019

@vmarchaud

This comment has been minimized.

Copy link
Contributor Author

vmarchaud commented Mar 13, 2019

I believe i've addressed every issue

@mayurkale22
Copy link
Collaborator

mayurkale22 left a comment

Please include Redis and IORedis in supported list of plugins -> https://github.com/census-instrumentation/opencensus-node#plugins

@@ -0,0 +1,17 @@
# OpenCensus ioredis Instrumentation for Node.js

This comment has been minimized.

@mayurkale22

mayurkale22 Mar 13, 2019

Collaborator

Optional : s/ioredis/IORedis?

@vmarchaud

This comment has been minimized.

Copy link
Contributor Author

vmarchaud commented Mar 14, 2019

@mayurkale22 Are you sure to add them in the global readme even though they are not currently enabled with the latest version ? We would need a separate PR to add them into instrumentation-all package so i believe this change should be done in the separate PR

@vmarchaud vmarchaud force-pushed the vmarchaud:ioredis-plugin branch from aca471f to b2b9a13 Mar 14, 2019

@mayurkale22

This comment has been minimized.

Copy link
Collaborator

mayurkale22 commented Mar 14, 2019

@mayurkale22 Are you sure to add them in the global readme even though they are not currently enabled with the latest version ? We would need a separate PR to add them into instrumentation-all package so i believe this change should be done in the separate PR

Makes sense 👍

@mayurkale22 mayurkale22 added the plugin label Mar 14, 2019

@mayurkale22 mayurkale22 added this to the Release 0.0.10 milestone Mar 14, 2019

@mayurkale22

This comment has been minimized.

Copy link
Collaborator

mayurkale22 commented Mar 14, 2019

Thanks for the contribution again!

@mayurkale22 mayurkale22 merged commit fa110c4 into census-instrumentation:master Mar 14, 2019

29 checks passed

ci/circleci: node10 Your tests passed on CircleCI!
Details
ci/circleci: node11 Your tests passed on CircleCI!
Details
ci/circleci: node6 Your tests passed on CircleCI!
Details
ci/circleci: node8 Your tests passed on CircleCI!
Details
cla/google All necessary CLAs are signed
security/snyk - examples/grpc/package.json (mayurkale22) No manifest changes detected
security/snyk - package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-core/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-example-automatic-tracing/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-exporter-instana/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-exporter-jaeger/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-exporter-ocagent/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-exporter-prometheus/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-exporter-stackdriver/package.json (mayurkale22) No new issues
Details
security/snyk - packages/opencensus-exporter-zipkin/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-exporter-zpages/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-instrumentation-all/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-instrumentation-grpc/package.json (mayurkale22) No new issues
Details
security/snyk - packages/opencensus-instrumentation-http/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-instrumentation-http2/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-instrumentation-https/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-instrumentation-mongodb/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-nodejs/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-propagation-b3/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-propagation-binaryformat/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-propagation-jaeger/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-propagation-stackdriver/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-propagation-tracecontext/package.json (mayurkale22) No manifest changes detected
security/snyk - packages/opencensus-resource-util/package.json (mayurkale22) No manifest changes detected
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.