Skip to content

Conversation

@smaddock
Copy link
Contributor

This PR modifies how Monolog context and extras arrays behave, and adds specific behavior for exceptions.

General Attributes

Per PSR-3:

[The context array] is meant to hold any extraneous information that does not fit well in a string. The array can contain anything. Implementors MUST ensure they treat context data with as much lenience as possible. A given value in the context MUST NOT throw an exception nor raise any php error, warning or notice.

Per Monolog:

The second way is to add extra data for all records by using a processor. Processors can be any callable. They will get the record as parameter and must return it after having eventually changed the extra part of it.

Per OTel Spec Logs Data Model:

[The Attributes Field is] additional information about the specific event occurrence. Unlike the Resource field, which is fixed for a particular source, Attributes can vary for each occurrence of the event coming from the same source. Can contain information about the request context (other than Trace Context Fields). This field is optional.

These descriptions indicate the intent of the context array of PSR-3, and to a lesser extent, the extras array of Monolog, has the same intended function as the Log Record Attributes of OpenTelemetry.

While identifying the need to prevent breaking existing implementations of the OpenTelemetry Monolog Handler, we suggest adding OTEL_PHP_MONOLOG_ATTRIB_MODE as a runtime configuration option to switch between the current behavior that prioritizes PSR-3 flexibility and safety, and a new behavior that prioritizes the OpenTelemetry spec and SemConv.

Examples are in the README.md.

Exceptions

Per PSR-3:

If an Exception object is passed in the context data, it MUST be in the 'exception' key. Logging exceptions is a common pattern and this allows implementors to extract a stack trace from the exception when the log backend supports it. Implementors MUST still verify that the 'exception' key is actually an Exception before using it as such, as it MAY contain anything.

Per the OTel Spec:

Additional information about errors and/or exceptions that are associated with a log record MAY be included in the structured data in the Attributes section of the record. If included, they MUST follow the OpenTelemetry semantic conventions for exception-related attributes.

Per the OTel SemConv:

Exceptions SHOULD be recorded as attributes on the LogRecord passed to the Logger emit operations. Exceptions MAY be recorded on "logs" or "events" depending on the context.

These descriptions indicate that exceptions included in the context array under the exception key should be treated differently than other general keys. This behavior is also included in this PR, regardless of the attributes mode.

@smaddock smaddock requested a review from a team as a code owner November 24, 2025 21:48
@codecov
Copy link

codecov bot commented Nov 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.63%. Comparing base (42f350a) to head (760a307).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #467      +/-   ##
============================================
+ Coverage     73.42%   81.63%   +8.21%     
- Complexity      134     2192    +2058     
============================================
  Files            11      159     +148     
  Lines           459     8238    +7779     
============================================
+ Hits            337     6725    +6388     
- Misses          122     1513    +1391     
Flag Coverage Δ
Aws 93.37% <ø> (?)
Context/Swoole 0.00% <ø> (?)
Exporter/Instana 49.80% <ø> (?)
Instrumentation/AwsSdk 82.00% <ø> (?)
Instrumentation/CakePHP 20.42% <ø> (?)
Instrumentation/CodeIgniter 79.31% <ø> (+0.31%) ⬆️
Instrumentation/Curl 87.15% <ø> (?)
Instrumentation/Doctrine 92.82% <ø> (?)
Instrumentation/ExtAmqp 88.80% <ø> (?)
Instrumentation/Guzzle 76.25% <ø> (?)
Instrumentation/HttpAsyncClient 78.94% <ø> (?)
Instrumentation/IO 0.00% <ø> (ø)
Instrumentation/Laravel 71.65% <ø> (?)
Instrumentation/MongoDB 76.84% <ø> (?)
Instrumentation/OpenAIPHP 86.71% <ø> (?)
Instrumentation/PDO 85.67% <ø> (?)
Instrumentation/PostgreSql 91.36% <ø> (?)
Instrumentation/Psr14 77.41% <ø> (?)
Instrumentation/Psr15 89.74% <ø> (?)
Instrumentation/Psr16 97.43% <ø> (?)
Instrumentation/Psr18 79.41% <ø> (+1.94%) ⬆️
Instrumentation/Psr3 67.70% <ø> (?)
Instrumentation/Psr6 97.56% <ø> (?)
Instrumentation/ReactPHP 99.41% <ø> (?)
Instrumentation/Session 94.28% <ø> (-0.24%) ⬇️
Instrumentation/Slim 84.21% <ø> (?)
Instrumentation/Yii 83.33% <ø> (?)
Logs/Monolog 100.00% <100.00%> (?)
Propagation/CloudTrace 90.69% <ø> (+0.92%) ⬆️
Propagation/Instana 98.07% <ø> (?)
Propagation/ServerTiming 94.73% <ø> (?)
Propagation/TraceResponse 94.73% <ø> (?)
ResourceDetectors/Azure 91.66% <ø> (?)
ResourceDetectors/Container 93.02% <ø> (ø)
ResourceDetectors/DigitalOcean 100.00% <ø> (?)
Sampler/RuleBased 32.14% <ø> (?)
Sampler/Xray 78.38% <ø> (?)
Shims/OpenTracing 92.99% <ø> (?)
SqlCommenter 95.58% <ø> (?)
Symfony 88.14% <ø> (?)
Utils/Test 87.79% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/Logs/Monolog/src/Handler.php 100.00% <100.00%> (ø)

... and 153 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 42f350a...760a307. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@brettmc
Copy link
Contributor

brettmc commented Nov 25, 2025

I think this makes sense. Can the non-default mode (OTEL_PHP_MONOLOG_ATTRIB_MODE=otel) be tested ?

@smaddock
Copy link
Contributor Author

I think this makes sense. Can the non-default mode (OTEL_PHP_MONOLOG_ATTRIB_MODE=otel) be tested ?

I copied the behavior from the PSR-3 Instrumentation library, which appears to be the only instance of ConfigurationResolver in use, but the phpt coverage tests for that library have a bunch of failures which makes it appear that the ConfigurationResolver related lines are not tested (not sure if they actually are or not). All that to say I'll need to do some trial and error to figure out the unit testing.

@smaddock
Copy link
Contributor Author

Tests added. CI failures are for Symphony, unrelated to this PR.

switch (self::$mode) {
case self::MODE_PSR3:
if (!is_scalar($attribute)) {
$attribute = json_encode($attribute);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth adding a throw on try/catch with JSON_THROW_ON_ERROR here so that any potential encoding issues can be handled?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should it be handled? Fallback to serialize?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also haven't checked if json_encode is even needed or if setAttribute handles it upstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants