Skip to content
This repository was archived by the owner on Jun 17, 2021. It is now read-only.

Support for the node_redis framework - #73

Open
a526672351 wants to merge 18 commits into
SkyAPM:masterfrom
a526672351:master
Open

Support for the node_redis framework#73
a526672351 wants to merge 18 commits into
SkyAPM:masterfrom
a526672351:master

Conversation

@a526672351

Copy link
Copy Markdown
Member

No description provided.

@ascrutae
ascrutae self-requested a review March 17, 2019 11:52
@ascrutae ascrutae added this to the 1.0.1 milestone Mar 17, 2019
@ascrutae ascrutae added the enhancement New feature or request label Mar 17, 2019
Comment thread modules/nodejs-agent/lib/plugins/redis/index.js Outdated
Tags.DB_TYPE.tag(span, "Redis");
Tags.DB_INSTANCE.tag(span, connection.selected_db);
Tags.DB_STATEMENT.tag(span, command_obj.command);
contextManager.finishSpan(span);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do you finish span in this time? The cost time of Redis span is incorrect if you finish the span in this time. The cost of real Redis span is the cost of executing Redis command.

@kezhenxu94 kezhenxu94 May 25, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do you finish span in this time? The cost time of Redis span is incorrect if you finish the span in this time. The cost of real Redis span is the cost of executing Redis command.

Hi @ascrutae , seems this comment is addressed, is there anything else that needs to be updated? Let me continue this if there is any

@zouyx

zouyx commented Mar 18, 2019

Copy link
Copy Markdown
Contributor

which node_module will this plugin support?

@a526672351

Copy link
Copy Markdown
Member Author

which node_module will this plugin support?

https://www.npmjs.com/package/redis

@zouyx

zouyx commented Mar 18, 2019

Copy link
Copy Markdown
Contributor

using ioredis now
have checked the document, it based on your node module.
so it will work theoretically

@a526672351

Copy link
Copy Markdown
Member Author

using ioredis now
have checked the document, it based on your node module.
so it will work theoretically

ioredis is not supported in this time.

@zouyx

zouyx commented Mar 18, 2019

Copy link
Copy Markdown
Contributor

my mistake, there is wrong reading for ioredis document.

@ascrutae ascrutae modified the milestones: 1.1.0, 1.2.0 Mar 19, 2019

@ascrutae ascrutae left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Any update?

@ascrutae
ascrutae requested a review from wu-sheng August 23, 2019 09:22

@ascrutae ascrutae left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Any update?

@wu-sheng wu-sheng removed this from the 1.2.0 milestone Mar 3, 2020
@kezhenxu94
kezhenxu94 requested a review from ascrutae May 25, 2020 14:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants