Skip to content

ioredis and redis DB semantic conventions#167

Merged
obecny merged 2 commits intoopen-telemetry:masterfrom
naseemkullah:ioredis-semantic-conventions
Aug 13, 2020
Merged

ioredis and redis DB semantic conventions#167
obecny merged 2 commits intoopen-telemetry:masterfrom
naseemkullah:ioredis-semantic-conventions

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>
@naseemkullah naseemkullah force-pushed the ioredis-semantic-conventions branch from 860ade1 to 524979b Compare August 10, 2020 12:55
@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