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

feat(agent): Implements log decorating #733

Merged
merged 15 commits into from
Sep 27, 2023
Merged

feat(agent): Implements log decorating #733

merged 15 commits into from
Sep 27, 2023

Conversation

mfulb
Copy link
Contributor

@mfulb mfulb commented Sep 19, 2023

Adds linking metadata to the Monolog LogRecord by using the 'extra' array element to store a string containing the log decorating linking metadata.

bduranleau-nr and others added 3 commits September 11, 2023 15:11
Adds linking metadata to the Monolog LogRecord by using the 'extra'
array element to store a string containing the log decorating linking
metadata.
@lavarou lavarou changed the title chore(agent): Implements log decorating feat(agent): Implements log decorating Sep 19, 2023
@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2023

Codecov Report

Merging #733 (0d136b4) into dev (0a25ba7) will increase coverage by 0.00%.
Report is 2 commits behind head on dev.
The diff coverage is 82.05%.

❗ Current head 0d136b4 differs from pull request most recent head 8e61b9f. Consider uploading reports for the commit 8e61b9f to get more accurate results

@@           Coverage Diff           @@
##              dev     #733   +/-   ##
=======================================
  Coverage   78.21%   78.22%           
=======================================
  Files         188      188           
  Lines       25994    26030   +36     
=======================================
+ Hits        20332    20362   +30     
- Misses       5662     5668    +6     
Flag Coverage Δ
agent-for-php-7.0 77.01% <26.31%> (-0.08%) ⬇️
agent-for-php-7.1 76.72% <26.31%> (-0.08%) ⬇️
agent-for-php-7.2 77.24% <88.88%> (+0.01%) ⬆️
agent-for-php-7.3 77.26% <88.88%> (+0.01%) ⬆️
agent-for-php-7.4 77.04% <88.88%> (+0.01%) ⬆️
agent-for-php-8.0 77.13% <88.88%> (+0.01%) ⬆️
agent-for-php-8.1 77.09% <88.88%> (+0.01%) ⬆️
agent-for-php-8.2 76.86% <88.88%> (+0.01%) ⬆️

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

Files Coverage Δ
agent/php_newrelic.h 100.00% <ø> (ø)
agent/php_nrini.c 83.61% <ø> (ø)
agent/php_txn.c 84.53% <100.00%> (+0.16%) ⬆️
axiom/nr_txn.c 94.55% <100.00%> (-0.01%) ⬇️
axiom/nr_txn.h 78.94% <ø> (ø)
agent/php_minit.c 65.88% <60.00%> (-0.12%) ⬇️
agent/lib_monolog.c 69.78% <81.48%> (+3.71%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

These tests are not actually for PHP 5 only but are simply run
with CLM disabled so a #skipif is not required.
The decorating processor function was being created multiple times if
more than one handler was pushed.  This was due to the scoped name
of the function (in lowercase) was not being used in the search to see
if it had been previously created.  A new integration test was added
to check the use case where more than one handler is being used.

The handler is now checked to see if it supported the pushProcessor()
functionality and if it does not then a warning is generated to notify
the user messages sent to that handler will not be decorated.
@mfulb mfulb marked this pull request as ready for review September 26, 2023 14:19
@jamesmehorter
Copy link

Hi, just a curious user here.. we've been eagerly awaiting PHP agent support for logging 'extra' (aka context) data along with each log entry (similar to how the java agent does it). Does this PR accomplish that?

axiom/tests/test_txn.c Outdated Show resolved Hide resolved
Copy link
Member

@lavarou lavarou left a comment

Choose a reason for hiding this comment

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

LGTM! 👍
P.S. I didn't review all integration test updates - just the monolog ones.

@bduranleau-nr
Copy link
Contributor

Are there any tests (unit or integration) to verify that entity.name is properly uri/url encoded?

Copy link
Contributor

@bduranleau-nr bduranleau-nr left a comment

Choose a reason for hiding this comment

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

👍

@mfulb mfulb merged commit 3023054 into dev Sep 27, 2023
@mfulb mfulb deleted the msf/apm-log-decorating branch September 27, 2023 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants