[Laravel] Add opt-in context flattening for faceted log search#560
[Laravel] Add opt-in context flattening for faceted log search#560nickmarden wants to merge 2 commits into
Conversation
Restores the OTEL_PHP_LARAVEL_LOG_ATTRIBUTES_FLATTEN feature from open-telemetry#491, now correctly layered on top of open-telemetry#511's structured-array default. - Default (no env var): context stored as structured array attribute, preserving the behaviour introduced in open-telemetry#511. - Opt-in (OTEL_PHP_LARAVEL_LOG_ATTRIBUTES_FLATTEN=true): context fields spread as individual top-level OTLP attributes using dot notation for nested keys (e.g. http.method, http.path), enabling faceted search in backends such as SigNoz, Jaeger, and Grafana. Adds unit tests covering both modes, exception extraction, Stringable normalisation, null filtering, and nested dot-notation expansion.
|
Thanks for opening your first pull request! If you haven't yet signed our Contributor License Agreement (CLA), then please do so that we can accept your contribution. A link should appear shortly in this PR if you have not already signed one. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #560 +/- ##
============================================
+ Coverage 80.45% 81.09% +0.63%
+ Complexity 1669 1521 -148
============================================
Files 116 93 -23
Lines 6262 5431 -831
============================================
- Hits 5038 4404 -634
+ Misses 1224 1027 -197 Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
ChrisLightfootWild
left a comment
ChrisLightfootWild
left a comment
There was a problem hiding this comment.
Thanks for re-creating this with a comprehensive write-up 💪
There's a failure in the pipeline, if you could take a look:
1) tests/Unit/Watchers/LogWatcherTest.php (class_definition)
Remove the space between `class` and `()` in the anonymous class instantiation, which the class_definition fixer rule requires.
|
@ChrisLightfootWild I think I fixed the issue that you highlighted in your previous comment, but CI is generally red for reasons unrelated to this PR AFAICT. Do you want me to take a stab at getting CI green (which would be a lot of scope creep), or is this PR OK as-is? |
|
@nickmarden, why do we need the flattening in the first place? I understand the original problem with the entire context being stored as a single string. However, after #511, the attributes are exposed as properties under the root Because of that, I am wondering what additional benefit flattening provides here, if the faceted search is working now. |
Apologies for my delayed response, I'm just back from vacation. Generally happy to delegate unrelated CI failures to separate PRs. |
@andrii-androshchuk I wasn't under the impression that ALL components of the context would be surfaced as attributes in dotted notation such as |
@andrii-androshchuk Thanks for the question — @arthurmartelli and I looked into this carefully. After #511, So The flattening this PR adds explicitly emits Attached are SigNoz screenshots showing The surfaced top-level attribute 
And the current 
My conclusion: I think we should still merge this to support the originally-intended use case. |
3 participants
Description
This PR restores the context-flattening feature from #491 (which was reverted in #559 due to a conflict with #511), now correctly layered on top of the structured-array default that #511 introduced.
Background
PR #491 identified a real observability problem: the Laravel
LogWatcherwas JSON-encoding the entire log context into a singlecontextstring attribute, making it impossible to filter by specific values in backends like SigNoz, Jaeger, or Grafana. For example, given context{"http": {"method": "GET", "path": "/users"}, "user_id": "123"}, there was no way to queryhttp.method = "GET"as a facet.PR #491 solved this with an opt-in
OTEL_PHP_LARAVEL_LOG_ATTRIBUTES_FLATTENflag that spreads context fields as individual OTLP attributes using dot notation for nested keys:What went wrong with #491
When #491 was merged, it conflicted with #511, which had already changed the default
contexthandling from a JSON-encoded string to a structured array (->setAttribute('context', $context)). The #491 code regressed that improvement by reverting the non-flatten path back tojson_encode().PR #559 reverted #491 entirely to restore the #511 behaviour, but that also lost the flattening feature.
What this PR does
This PR re-introduces the flattening feature cleanly on top of #511:
->setAttribute('context', ['http' => ['method' => 'GET'], 'user_id' => '123']).OTEL_PHP_LARAVEL_LOG_ATTRIBUTES_FLATTEN=true): context fields are spread as individual top-level OTLP attributes usingIlluminate\Support\Arr::dot(), enabling faceted search in observability backends.The non-flatten default path is identical to what #511 shipped. The flatten path is additive and off by default.
Changes
src/Watchers/LogWatcher.php: addsOTEL_PHP_LARAVEL_LOG_ATTRIBUTES_FLATTENconstant, reads it viaConfigurationResolverin the constructor, and branches on it inrecordLog(). AnormalizeValue()helper ensures OTLP-compatible scalar types (castingStringableobjects to string, JSON-encoding anything else).tests/Unit/Watchers/LogWatcherTest.php: 8 unit tests covering default structured-array mode, flatten mode, nested dot-notation expansion, exception extraction,Stringablenormalisation, and null filtering.Backward compatibility
OTEL_PHP_LARAVEL_LOG_ATTRIBUTES_FLATTENis off by default; existing deployments are unaffected.