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

ioredis and redis DB semantic conventions #167

Merged

Conversation

naseemkullah
Copy link
Member

@naseemkullah naseemkullah commented Aug 5, 2020

towards #116

Want to do this one first, will likely follow up with more.

N.B. Now nothing in attributes points to which specific library (e.g.: ioredis) is being traced.

@naseemkullah naseemkullah requested a review from a team August 5, 2020 03:06
@naseemkullah
Copy link
Member Author

typedocs 😍
image

@naseemkullah
Copy link
Member Author

What to do about

test/userInteraction.nozone.test.ts:48:39 - error TS2345: Argument of type 'ZoneContextManager' is not assignable to parameter of type 'ContextManager'.
  The types returned by 'active()' are incompatible between these types.
    Type 'import("/root/project/plugins/web/opentelemetry-plugin-user-interaction/node_modules/@opentelemetry/context-base/build/src/context").Context' is not assignable to type 'import("/root/project/plugins/web/opentelemetry-plugin-user-interaction/node_modules/@opentelemetry/api/node_modules/@opentelemetry/context-base/build/src/context").Context'.
      Types have separate declarations of a private property '_currentContext'.

48       context.setGlobalContextManager(contextManager);
                                         ~~~~~~~~~~~~~~

test/userInteraction.test.ts:51:39 - error TS2345: Argument of type 'ZoneContextManager' is not assignable to parameter of type 'ContextManager'.

51       context.setGlobalContextManager(contextManager);

?

@naseemkullah naseemkullah changed the title ioredis DB semantic conventions ioredis and redis DB semantic conventions Aug 5, 2020
@naseemkullah
Copy link
Member Author

Now includes redis as well...

@dyladan
Copy link
Member

dyladan commented Aug 5, 2020

I'm not sure about the build failure, but my best guess is that you bumped the core versions for some, but not all, of the main packages. i would revert those changes as they are not relevant here.

@naseemkullah
Copy link
Member Author

naseemkullah commented Aug 5, 2020

I'm not sure about the build failure, but my best guess is that you bumped the core versions for some, but not all, of the main packages. i would revert those changes as they are not relevant here.

Very well, I'll let renovate bot do its job!

@naseemkullah naseemkullah force-pushed the ioredis-semantic-conventions branch 2 times, most recently from 1c1fe90 to 24d777b Compare August 5, 2020 12:34
@naseemkullah
Copy link
Member Author

Still failing despite leaving unrelated packages alone


> @opentelemetry/plugin-user-interaction@0.9.0 version:update /root/project/plugins/web/opentelemetry-plugin-user-interaction
> node ../../../scripts/version-update.js

test/userInteraction.nozone.test.ts:48:39 - error TS2345: Argument of type 'ZoneContextManager' is not assignable to parameter of type 'ContextManager'.
  The types returned by 'active()' are incompatible between these types.
    Type 'import("/root/project/plugins/web/opentelemetry-plugin-user-interaction/node_modules/@opentelemetry/context-base/build/src/context").Context' is not assignable to type 'import("/root/project/plugins/web/opentelemetry-plugin-user-interaction/node_modules/@opentelemetry/api/node_modules/@opentelemetry/context-base/build/src/context").Context'.
      Types have separate declarations of a private property '_currentContext'.

48       context.setGlobalContextManager(contextManager);
                                         ~~~~~~~~~~~~~~

test/userInteraction.test.ts:51:39 - error TS2345: Argument of type 'ZoneContextManager' is not assignable to parameter of type 'ContextManager'.

51       context.setGlobalContextManager(contextManager);
                                         ~~~~~~~~~~~~~~


Found 2 errors.

Signed-off-by: Naseem <naseem@transit.app>
@naseemkullah naseemkullah force-pushed the ioredis-semantic-conventions branch 2 times, most recently from 1c68ea4 to 860ade1 Compare August 10, 2020 12:54
Signed-off-by: Naseem <naseem@transit.app>
@codecov
Copy link

codecov bot commented Aug 10, 2020

Codecov Report

Merging #167 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #167      +/-   ##
==========================================
- Coverage   94.11%   94.06%   -0.05%     
==========================================
  Files          74       72       -2     
  Lines        3583     3558      -25     
  Branches      387      385       -2     
==========================================
- Hits         3372     3347      -25     
  Misses        211      211              
Impacted Files Coverage Δ
node/opentelemetry-plugin-redis/src/utils.ts 90.90% <0.00%> (ø)
node/opentelemetry-plugin-ioredis/src/utils.ts 85.36% <0.00%> (ø)
node/opentelemetry-plugin-ioredis/src/ioredis.ts 100.00% <0.00%> (ø)
node/opentelemetry-plugin-redis/test/redis.test.ts 93.26% <0.00%> (ø)
.../opentelemetry-plugin-ioredis/test/ioredis.test.ts 95.30% <0.00%> (ø)
node/opentelemetry-plugin-redis/src/enums.ts
node/opentelemetry-plugin-ioredis/src/enums.ts

Copy link
Member

@vmarchaud vmarchaud left a comment

Choose a reason for hiding this comment

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

lgtm

@obecny obecny merged commit 674b384 into open-telemetry:master Aug 13, 2020
@obecny obecny added the enhancement New feature or request label Sep 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants