Repository navigation
Implement span suppression strategies - #1599
Conversation
brettmc
left a comment
brettmc
left a comment
There was a problem hiding this comment.
This looks pretty solid. There's a lot of configuration, it seems like this will need to use declarative configuration, but that can be a follow-up change.
|
I'm happy to accept this as-is and complete it incrementally, if you don't have much time. If you're happy with that, can you add |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1599 +/- ##
============================================
+ Coverage 68.24% 68.36% +0.12%
- Complexity 2919 2978 +59
============================================
Files 435 447 +12
Lines 8876 9008 +132
============================================
+ Hits 6057 6158 +101
- Misses 2819 2850 +31
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| new SemanticConvention('span.http.client', SpanKind::KIND_CLIENT, ['http.request.method', 'server.address', 'server.port', 'url.full'], ['network.peer.address', 'network.peer.port', 'error.type', 'http.request.body.size', 'http.request.header', 'http.request.method_original', 'http.request.resend_count', 'http.request.size', 'http.response.body.size', 'http.response.header', 'http.response.size', 'http.response.status_code', 'network.protocol.name', 'network.protocol.version', 'network.transport', 'user_agent.original', 'user_agent.synthetic.type', 'url.scheme', 'url.template']), | ||
| new SemanticConvention('span.http.server', SpanKind::KIND_SERVER, ['http.request.method', 'url.path', 'url.scheme'], ['network.peer.address', 'network.peer.port', 'error.type', 'http.request.body.size', 'http.request.method_original', 'http.request.size', 'http.response.body.size', 'http.response.header', 'http.response.size', 'http.response.status_code', 'network.protocol.name', 'network.protocol.version', 'network.transport', 'user_agent.synthetic.type', 'client.address', 'client.port', 'http.request.header', 'http.route', 'network.local.address', 'network.local.port', 'user_agent.original', 'server.address', 'server.port', 'url.query']), |
There was a problem hiding this comment.
| new SemanticConvention('span.http.client', SpanKind::KIND_CLIENT, ['http.request.method', 'server.address', 'server.port', 'url.full'], ['network.peer.address', 'network.peer.port', 'error.type', 'http.request.body.size', 'http.request.header', 'http.request.method_original', 'http.request.resend_count', 'http.request.size', 'http.response.body.size', 'http.response.header', 'http.response.size', 'http.response.status_code', 'network.protocol.name', 'network.protocol.version', 'network.transport', 'user_agent.original', 'user_agent.synthetic.type', 'url.scheme', 'url.template']), | |
| new SemanticConvention('span.http.server', SpanKind::KIND_SERVER, ['http.request.method', 'url.path', 'url.scheme'], ['network.peer.address', 'network.peer.port', 'error.type', 'http.request.body.size', 'http.request.method_original', 'http.request.size', 'http.response.body.size', 'http.response.header', 'http.response.size', 'http.response.status_code', 'network.protocol.name', 'network.protocol.version', 'network.transport', 'user_agent.synthetic.type', 'client.address', 'client.port', 'http.request.header', 'http.route', 'network.local.address', 'network.local.port', 'user_agent.original', 'server.address', 'server.port', 'url.query']), | |
| new SemanticConvention('span.http.client', SpanKind::KIND_CLIENT, ['http.request.method', 'server.address', 'server.port', 'url.full'], ['network.peer.address', 'network.peer.port', 'error.type', 'http.request.body.size', 'http.request.header.*', 'http.request.method_original', 'http.request.resend_count', 'http.request.size', 'http.response.body.size', 'http.response.header.*', 'http.response.size', 'http.response.status_code', 'network.protocol.name', 'network.protocol.version', 'network.transport', 'user_agent.original', 'user_agent.synthetic.type', 'url.scheme', 'url.template']), | |
| new SemanticConvention('span.http.server', SpanKind::KIND_SERVER, ['http.request.method', 'url.path', 'url.scheme'], ['network.peer.address', 'network.peer.port', 'error.type', 'http.request.body.size', 'http.request.method_original', 'http.request.size', 'http.response.body.size', 'http.response.header.*', 'http.response.size', 'http.response.status_code', 'network.protocol.name', 'network.protocol.version', 'network.transport', 'user_agent.synthetic.type', 'client.address', 'client.port', 'http.request.header.*', 'http.route', 'network.local.address', 'network.local.port', 'user_agent.original', 'server.address', 'server.port', 'url.query']), |
Implementation does not handle template types yet.
There was a problem hiding this comment.
Must also add general attributes https://opentelemetry.io/docs/specs/semconv/general/attributes/
The attributes described in this section are not specific to a particular operation but rather generic. They may be used in any Span they apply to.
There was a problem hiding this comment.
Seems like I forgot to include inherited attributes when generating this resolver (for example db.system.name is missing in all span.db.*.client semconvs), should be fixed as a follow-up.
| */ | ||
| interface SpanSuppressionStrategy | ||
| { | ||
| public function getSuppressor(string $name, ?string $version, ?string $schemaUrl): SpanSuppressor; |
There was a problem hiding this comment.
Could accept InstrumentationScopeInterface instead.
| /** | ||
| * @param int<0, 4> $spanKind | ||
| */ | ||
| public function resolveSuppression(int $spanKind, array $attributes): SpanSuppression; |
There was a problem hiding this comment.
Could accept AttributesInterface instead.
Note: SemanticConventionSuppressor should still operate on the underlying ::toArray() to avoid unnecessary method calls.
b0205b3 to
9b3653c
Compare
|
Is there any movement in this at all? Currently have a lot of pain when using some of the symfony / psr auto instrumentation bundles for HTTP requests, hope this will alleviate the problem. See a lot of nested spans that are identical in name that are sometimes 5/6 levels deep. 
|
Noop: does not suppress anything SpanKind: suppresses nested spans with the same span kind (except for internal) SemConv: attempts to guess and suppress the semantic convention that is represented by the span
9b3653c to
0afbf7e
Compare
|
Does this PR expose any way to configure which suppression strategy is used by the SDK when used via autoinstrumentation bundles? Was expecting to see some environment variable configuration, but haven't seen any. |
No. I think that will be a follow-up, which is called out in #1579. I think it's too complicated to work with env vars, and so would require YAML-based declarative config (happy to be proven wrong, though). So step 1, let's get it in, then do some field testing that it works, then make it more useful/accessible. |
brettmc
changed the title
github-project-automation
Bot
moved this from In Progress
to Done
in PHP Prioritized Backlog
4 participants
Alternative to #1583
Span suppression is implemented in the SDK -> instrumentation does not need to handle suppression.
(Everything besides
SemanticConventionandSemanticConventionResolvercould be moved to the SDK.)Provides three span suppression strategies:
Noop: does not suppress anything
SpanKind: suppresses nested spans with the same span kind (except for internal)
SemConv: attempts to resolve the semantic convention based on span attributes, otherwise falls back to span kind suppression