Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[proposal] servicegraph support virtual/peer node #17196

Closed
JaredTan95 opened this issue Dec 22, 2022 · 7 comments
Closed

[proposal] servicegraph support virtual/peer node #17196

JaredTan95 opened this issue Dec 22, 2022 · 7 comments
Labels
enhancement New feature or request needs triage New item requiring triage

Comments

@JaredTan95
Copy link
Member

JaredTan95 commented Dec 22, 2022

Component(s)

processor/servicegraph

Is your feature request related to a problem? Please describe.

as #9232 (comment) designed, the current processor discards the call relationship when processing the following scenarios:

  • It is requested by the browser to the backend, but the browser side does not integrate OTEL/sdk, and the backend integrates otel. So, the processor only received backend server span and will drop it from the cache when expired.

  • The backend requests a service for an external address(like cloud service), and the backend integrates OTEL/sdk, but the external address does not integrate OTEL.

Describe the solution you'd like

so, I propose to patch edge before the processor discards these spans, depending on the kind of span in the cache:

finally , the metrics geanerated would be like:

traces_service_graph_request_total{client="user",server="B"} 10

traces_service_graph_request_total{client="a",server="http://example.com/hello-world"} 1

traces_service_graph_request_total{client="a",server="grpc.hello_world"} 1

traces_service_graph_request_total{client="a",server="10.10.10.1:8080"} 1

Describe alternatives you've considered

No response

Additional context

No response

@JaredTan95 JaredTan95 added enhancement New feature or request needs triage New item requiring triage labels Dec 22, 2022
@github-actions
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@JaredTan95
Copy link
Member Author

I already finished it and tested in my fork repo. if the proposal makes sense, I will contribute it upstream. :)

@mapno
Copy link
Contributor

mapno commented Dec 22, 2022

Hi! Thanks for the proposal, I like the two use-cases described here. They can be a nice addition to the servicegraph processor.

We can make the actual implementation a bit more generic, so it doesn't spill to the store pkg. We also need to make sure that the use-cases are included in the semantic conventions-ie. net.sock.peer.addr is meant to be used for those types of connections.

@JaredTan95
Copy link
Member Author

JaredTan95 commented Jan 3, 2023

We also need to make sure that the use-cases are included in the semantic conventions-ie. net.sock.peer.addr is meant to be used for those types of connections.

After careful consideration, I found that not every span attribute has a generic tag like net.sock.peer.addr.

for database peer node, it only db.url provided for us to get peer info.

@mapno
Copy link
Contributor

mapno commented Jan 4, 2023

After careful consideration, I found that not every span attribute has a generic tag like net.sock.peer.addr.

Are there any attributes in the semantic conventions that we can reliably use to extract the data in the use-cases you described?

@JaredTan95
Copy link
Member Author

Are there any attributes in the semantic conventions that we can reliably use to extract the data in the use-cases you described?

Unfortunately, it seems that this does not exist. :-(

@djaglowski
Copy link
Member

Closed by #17350

JaredTan95 added a commit to JaredTan95/opentelemetry-collector-contrib that referenced this issue Jul 3, 2024
codeboten pushed a commit that referenced this issue Jul 3, 2024
Previous contributions to servicegraph are as follows:
1. make a proposal and impl [`virtual node`
feature](#17196)
in
[PR](#17350)

2. [fix bugs and
enhancements](https://github.com/open-telemetry/opentelemetry-collector-contrib/pulls?q=is%3Apr+author%3AJaredTan95+label%3Aprocessor%2Fservicegraph+is%3Aclosed)

---------

Signed-off-by: Jared Tan <jian.tan@daocloud.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs triage New item requiring triage
Projects
None yet
Development

No branches or pull requests

3 participants