skywalking: use header's path for entry/exit span's operation name

[Shikugawa]

Signed-off-by: Shikugawa rei@tetrate.io

Commit Message: skywalking: use header's path for entry/exit span's operation name
Additional Description: In Apache SkyWalking, we have a usecase to use pathname in downstream header as child span's name. This feature adds a option to achieve this requirement.
Risk Level: Low
Testing: Unit
Docs Changes: N/A
Release Notes: Required
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

cc @wu-sheng

[repokitteh-read-only]

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #20452 was opened by Shikugawa.

see: more, trace.

[wbpcode]

Would you consider add new API to TracingSpan(cpp2sky) to override operation name and then, when we call the injectContext in the Envoy, we can override the operation name with the path by the new API.

Then all the skywalking users needn't to set use_path_as_child_span_name in the Router. 🤔

[wbpcode]

/assign

[Shikugawa]

Would you consider add new API to TracingSpan(cpp2sky) to override operation name and then, when we call the injectContext in the Envoy, we can override the operation name with the path by the new API.

Then all the skywalking users needn't to set use_path_as_child_span_name in the Router. thinking

@wbpcode Do we think that all of skywalking users want to use path name as operation name? This is why I added this option here. cc @wu-sheng

[wu-sheng]

Entry span, uses the path of downstream as operation name.
Exit span, uses the path of upstream request path.

This is my idea.

[wbpcode]

Would you consider add new API to TracingSpan(cpp2sky) to override operation name and then, when we call the injectContext in the Envoy, we can override the operation name with the path by the new API.
Then all the skywalking users needn't to set use_path_as_child_span_name in the Router. thinking

@wbpcode Do we think that all of skywalking users want to use path name as operation name? This is why I added this option here. cc @wu-sheng

Considering the implementation of skywalking agent (Java), I think it ok to use path as operation name for all the skywalking users by default. If some users have special needs, we should also consider extending skywalking tracer's configuration to provides additional options.
cc @wu-sheng

[Shikugawa]

Thank you for the discussions. I found that it makes sense that envoy ALWAYS use path name in skywalking tracer. As I described, we may not use arbitrary operation name here. But it is out of scope. Let me fix.

[Shikugawa]

I think that we should work after #20454 has been merged. It rewrites operation name to path in setStreamInfo.

[github-actions]

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

[wu-sheng]

@wbpcode I noticed there SkyWalking relative PRs have no update from maintainers, how should we process these?

[wbpcode]

@wbpcode I noticed there SkyWalking relative PRs have no update from maintainers, how should we process these?

Sorry. Now we have three pending skywalking related PR's, #20367, #20454 and this one.

It's LGTM overall for me for the #20367, only need to fix the CI. (cc @Shikugawa )
I have different idea about the implementation of #20454. But I will not stand the way.
This PR should be completed after #20454 as cc @Shikugawa said.