Skip to content

spring-cloud-gateway traceid does not transmit #3411#3446

Merged
wu-sheng merged 5 commits intoapache:masterfrom
hi-sb:master
Sep 11, 2019
Merged

spring-cloud-gateway traceid does not transmit #3411#3446
wu-sheng merged 5 commits intoapache:masterfrom
hi-sb:master

Conversation

@hi-sb
Copy link
Contributor

@hi-sb hi-sb commented Sep 9, 2019

skywalking-version:6.5.0

spring-gateway-version:2.1.2

Error Description:

Customize the filter, traceid does not transmit.

"ServerWebExchangeDecorator" is a transport chain,His structure should be like this:

 serverWebExchangeDecorator
    -----(ServerWebExchangeDecorator)delegate
	    ------(ServerWebExchangeDecorator)delegate
		  ------.....
		   -----(DefaultServerWebExchange)delegate

In the current source code, there is no deep search, but only the next level. When there are multiple custom filters, you get an error.

Repair method:

Look for "delegate" of "ServerWebExchangeDecorator" recursively until "DefaultServerWebExchange"

Please answer these questions before submitting pull request

  • Why submit this pull request?

  • Bug fix

  • New feature provided

  • Improve performance

  • Related issues


Bug fix

  • Bug description.

  • How to fix?


New feature or improvement

  • Describe the details and related test reports.

skywalking-version:6.5.0

spring-gateway-version:2.1.2

Error Description:

 Customize the filter, traceid does not transmit.

 "ServerWebExchangeDecorator" is a transport chain,His structure should be like this:

	 serverWebExchangeDecorator
	    -----(ServerWebExchangeDecorator)delegate
		    ------(ServerWebExchangeDecorator)delegate
			  ------.....
			   -----(DefaultServerWebExchange)delegate
  In the current source code, there is no deep search, but only the next level. When there are multiple custom filters, you get an error.

Repair method:

  Look for "delegate" of "ServerWebExchangeDecorator" recursively until "DefaultServerWebExchange"
spring-boot-starter-webflux-version:2.1.6

Error Description:

Customize the filter, traceid does not transmit.

"ServerWebExchangeDecorator" is a transport chain,His structure should be like this:

 serverWebExchangeDecorator
    -----(ServerWebExchangeDecorator)delegate
	    ------(ServerWebExchangeDecorator)delegate
		  ------.....
		   -----(DefaultServerWebExchange)delegate

In the current source code, there is no deep search, but only the next level. When there are multiple custom filters, you get an error.

Repair method:

Look for "delegate" of "ServerWebExchangeDecorator" recursively until "DefaultServerWebExchange"
@zhaoyuguang
Copy link
Member

Oh, I may understand, what do you think of using the recursive method, but I hope you add this scenario to the gateway testcase, to make the plugin test more realistic

@wu-sheng wu-sheng added the TBD To be decided later, need more discussion or input. label Sep 10, 2019
@hi-sb
Copy link
Contributor Author

hi-sb commented Sep 10, 2019

my test pull request:

SkyAPMTest/agent-auto-integration-testcases#88

@zhaoyuguang
Copy link
Member

zhaoyuguang commented Sep 10, 2019

CI not pass, please check checkStyle.xml

@hi-sb
Copy link
Contributor Author

hi-sb commented Sep 10, 2019

Modified style to meet "check style"

@zhaoyuguang
Copy link
Member

I had triggered the gateway and webflux auto tests, let's wait for the result

@SkyWalkingRobot
Copy link

Here is the test report and validate logs

@zhaoyuguang
Copy link
Member

Look like @component annotation cause more spans

@hi-sb
Copy link
Contributor Author

hi-sb commented Sep 10, 2019

@zhaoyuguang This should be another issue.We can't restrict other people from using this annotation.

@wu-sheng
Copy link
Member

This should be another issue.We can't restrict other people from using this annotation.

This is not an issue of this PR, but an issue for your test case.

@hi-sb
Copy link
Contributor Author

hi-sb commented Sep 10, 2019

@wu-sheng I mean, in actual development, there may be people who write as they do in use cases. We can't avoid such problems. But this problem does not fall within the scope of this repair.

@wu-sheng
Copy link
Member

The point is, I recommend you to add these spans to your verify files, as expected span. Because in tests, the Spring annotation plugin actives with your gateway plugin. Both of them are optional, the test framework only supports them both active or inactive.

@hi-sb
Copy link
Contributor Author

hi-sb commented Sep 10, 2019

I see what you mean. What you mean is that this annotation activates other plug-ins. I shouldn't do that in use cases.

@hi-sb
Copy link
Contributor Author

hi-sb commented Sep 10, 2019

@zhaoyuguang @wu-sheng Sorry,We just started using "skywalking".I'm not too familiar with the other plug-ins he owns. I updated my use case.

@zhaoyuguang
Copy link
Member

zhaoyuguang commented Sep 10, 2019

@zhaoyuguang @wu-sheng Sorry,We just started using "skywalking".I'm not too familiar with the other plug-ins he owns. I updated my use case.

Ha~, it doesn't matter, I just tested it,
in skywalking-agent-testcases/gateway-scenario/projectA/docker/startup.sh line5 add
rm -rf ${AGENT_FILE_PATH}/plugins/apm-spring-annotation-plugin-*.jar can work as we expect

@hi-sb
Copy link
Contributor Author

hi-sb commented Sep 10, 2019

@zhaoyuguang What else do I need to do?

@zhaoyuguang
Copy link
Member

@zhaoyuguang What else do I need to do?

Just remove apm-spring-annotation-plugin-*.jar can avoid this interceptor

@SkyWalkingRobot
Copy link

Here is the test report and validate logs

@apache apache deleted a comment from SkyWalkingRobot Sep 11, 2019
@apache apache deleted a comment from SkyWalkingRobot Sep 11, 2019
Copy link
Member

@zhaoyuguang zhaoyuguang left a comment

Choose a reason for hiding this comment

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

LGTM

@wu-sheng
Copy link
Member

/run e2e

@wu-sheng wu-sheng added agent Language agent related. bug Something isn't working and you are sure it's a bug! plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor. and removed TBD To be decided later, need more discussion or input. labels Sep 11, 2019
@wu-sheng
Copy link
Member

/run e2e

Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

Forward approve

@wu-sheng
Copy link
Member

@hi-sb Welcome to be 155th contributor. Welcome to the SkyWalking contributor team.

@wu-sheng wu-sheng merged commit 96b2baa into apache:master Sep 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Language agent related. bug Something isn't working and you are sure it's a bug! plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants