-
Notifications
You must be signed in to change notification settings - Fork 16
feat : add support for handling grpc request url #284
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
Conversation
Codecov Report
@@ Coverage Diff @@
## main #284 +/- ##
============================================
+ Coverage 79.08% 79.14% +0.06%
- Complexity 1231 1237 +6
============================================
Files 110 111 +1
Lines 4862 4881 +19
Branches 440 443 +3
============================================
+ Hits 3845 3863 +18
Misses 813 813
- Partials 204 205 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
|
||
| GrpcAttributeEnricher { | ||
| class = "org.hypertrace.traceenricher.enrichment.enrichers.GrpcAttributeEnricher" | ||
| dependencies = ["SpanTypeAttributeEnricher", "ApiBoundaryTypeAttributeEnricher"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is PROTOCOL and enriched attribute?
If yes, are these 2 dependencies good enough for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes PROTOCOL is enriched attributes, and these deps take care of both enriched attributes required by this enricher.
Ref :
| public static final String HEAD_EVENT_ID = "head.event.id"; | ||
| public static final String API_EXIT_CALLS_COUNT = "api.exit.calls.count"; | ||
| public static final String UNIQUE_API_NODES_COUNT = "unique.apis.count"; | ||
| public static final String GRPC_REQUEST_URL_FORMAT_DOTTED = "grpc.request.url.format.dotted"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can get rid of the .format.dotted part in both the attributes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
For RPC span, the span name is equivalent to Endpoint (URL) - https://github.com/open-telemetry/opentelemetry-specification/blob/3e380e249f60c3a5f68746f5e84d10195ba41a79/specification/trace/semantic_conventions/rpc.md#span-name
In cases if the convention is not followed, we would like to extend the platform to handle calc grpc request URL (& Endpoint) using a few other fallback attributes.
Secondly, we also want to enrich the URL based on client (Sent) or server (Recv) calls.
So, This PR,