Skip to content

Fix multiple requests for the same traceId in Tornado#209

Closed
YueLangsugar wants to merge 3 commits intoapache:masterfrom
YueLangsugar:master
Closed

Fix multiple requests for the same traceId in Tornado#209
YueLangsugar wants to merge 3 commits intoapache:masterfrom
YueLangsugar:master

Conversation

@YueLangsugar
Copy link
Copy Markdown

@YueLangsugar YueLangsugar commented May 26, 2022

  • Fix the problem of multiple requests for the same traceId in Tornado framework

@Superskyyy
Copy link
Copy Markdown
Member

Please update your pull request title and text body, it's not readable.

@Superskyyy
Copy link
Copy Markdown
Member

Please also update the CHANGELOG.md to reflect your fix intentions.

@Superskyyy Superskyyy added the bug Something isn't working label May 26, 2022
@Superskyyy Superskyyy added this to the 1.0.0 milestone May 26, 2022
@YueLangsugar YueLangsugar changed the title The context of obtaining span is modified, which will affect the remo… Fix multiple requests for the same traceId in Tornado May 27, 2022
@YueLangsugar
Copy link
Copy Markdown
Author

Please update your pull request title and text body, it's not readable.

ok, It's modified

@YueLangsugar
Copy link
Copy Markdown
Author

Please also update the CHANGELOG.md to reflect your fix intentions.

I've updated it.

Comment thread CHANGELOG.md Outdated
Copy link
Copy Markdown
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

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

This patch doesn't make sense, it fixes (maybe) issues in tornado but breaks many other things, @YueLangsugar did you read the reply from @Superskyyy in apache/skywalking#9133?

Co-authored-by: 吴晟 Wu Sheng <wu.sheng@foxmail.com>
@YueLangsugar
Copy link
Copy Markdown
Author

This patch doesn't make sense, it fixes (maybe) issues in tornado but breaks many other things, @YueLangsugar did you read the reply from @Superskyyy in apache/skywalking#9133?

I've read it, but I don't think it helps.

@kezhenxu94
Copy link
Copy Markdown
Member

This patch doesn't make sense, it fixes (maybe) issues in tornado but breaks many other things, @YueLangsugar did you read the reply from @Superskyyy in apache/skywalking#9133?

I've read it, but I don't think it helps.

Please, let's discuss and have a consensus first in apache/skywalking#9133 , I don't think you can modify something but "you don't know why". You face issues in Tornado plugin but you are modifying the agent core, this doesn't make sense to me, and it may bring side effects that you are unaware of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants