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(axiom): Add message segment functionality #991

Open
wants to merge 55 commits into
base: dev
Choose a base branch
from

Conversation

zsistla
Copy link
Contributor

@zsistla zsistla commented Dec 10, 2024

  1. Added message segment.
    Agent will now handle generic, external, datastore, and message segments.

  2. Added unit tests for the changes

  3. New functionality involves adding message specific attributes to segments/spans, generating message specific metrics, and honoring the message_tracer_parameters_enabled INI setting to disable/enable the attributes.

4)Things to keep in mind while reviewing:

  • the commits describe a lot of what is going on
  • Functionality-wise, message segment is a mix of external and datastore
  • nr_segment_message is the new file

zsistla and others added 23 commits October 23, 2024 14:24
Set segment type enum
Create nr_segment_message_t struct
Add to the nr_segment_typed_attributes_t union
* Create enums for message attributes attributes
* Add MESSAGE enum to nr_span_category_t span category
* create nr_span_event_set_message
create `nr_span_event_set_spankind`

for generic,http,datastore the logic of nr_span_event_set_category is overloaded because according agent-specs/Span-Events.md, span.kind could only ever be “client” for datastore and http spans  and empty for generic spans.  I debated extracting all the logic and making nr_span_event_set_spankind calls for those categories in nr_segment.c; however, since the span.kind values are always the same for those span categories, I left it in for now.  Message spans however, will call nr_span_event_set_spankind separately and nr_span_event_set_category will only be used to set the category.
add message_type to nr_segment_message_t to inform the spankind

since message_type is on nr_segment_message_t, give a default in case the message span is created but the nr_segment_message_t is not accessible

added function nr_populate_message_spans

added function nr_segment_set_message

added message segment handling to nr_segment_to_span_event
Add the message_tracer_segment_parameters_enabled value to opts for use in axiom.
Includes creating metrics and ending the message segment.
* test get/set spankind
* test get/set message span members
Sorry clang-format added some unrelated formatting.
Rollups happen in 2 places in nr_txn.c
nr_error_to_event
And
nr_txn_event_intrinsics
* fixed externalCallCount was getting set in nr_error_to_event  but not in nr_txn_event_intrinsics
* Added messageCallCount/messageDuration to both functions
Handle NULL/empty str and provide the `<unknown>` string.
@newrelic-php-agent-bot
Copy link

newrelic-php-agent-bot commented Dec 10, 2024

Test Suite Status Result
Multiverse 7/7 passing
SOAK 61/62 passing

ZNeumann and others added 17 commits December 10, 2024 08:01
* Create enums for message attributes attributes
* Add MESSAGE enum to nr_span_category_t span category
* create nr_span_event_set_message
create `nr_span_event_set_spankind`

for generic,http,datastore the logic of nr_span_event_set_category is overloaded because according agent-specs/Span-Events.md, span.kind could only ever be “client” for datastore and http spans  and empty for generic spans.  I debated extracting all the logic and making nr_span_event_set_spankind calls for those categories in nr_segment.c; however, since the span.kind values are always the same for those span categories, I left it in for now.  Message spans however, will call nr_span_event_set_spankind separately and nr_span_event_set_category will only be used to set the category.
add message_type to nr_segment_message_t to inform the spankind

since message_type is on nr_segment_message_t, give a default in case the message span is created but the nr_segment_message_t is not accessible

added function nr_populate_message_spans

added function nr_segment_set_message

added message segment handling to nr_segment_to_span_event
Add the message_tracer_segment_parameters_enabled value to opts for use in axiom.
Includes creating metrics and ending the message segment.
* test get/set spankind
* test get/set message span members
Sorry clang-format added some unrelated formatting.
Rollups happen in 2 places in nr_txn.c
nr_error_to_event
And
nr_txn_event_intrinsics
* fixed externalCallCount was getting set in nr_error_to_event  but not in nr_txn_event_intrinsics
* Added messageCallCount/messageDuration to both functions
Handle NULL/empty str and provide the `<unknown>` string.
@zsistla zsistla force-pushed the feat/message_segment branch from 3e448d0 to c8d2db1 Compare December 10, 2024 15:01
@codecov-commenter
Copy link

codecov-commenter commented Dec 10, 2024

Codecov Report

Attention: Patch coverage is 96.46018% with 8 lines in your changes missing coverage. Please review.

Project coverage is 78.76%. Comparing base (0d0f724) to head (c972a35).

Files with missing lines Patch % Lines
axiom/nr_segment_message.c 91.35% 7 Missing ⚠️
axiom/nr_span_event.c 98.52% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #991      +/-   ##
==========================================
+ Coverage   77.94%   78.76%   +0.81%     
==========================================
  Files         195      197       +2     
  Lines       26325    27289     +964     
==========================================
+ Hits        20520    21493     +973     
+ Misses       5805     5796       -9     
Flag Coverage Δ
agent-for-php-7.2 78.78% <96.44%> (?)
agent-for-php-7.3 78.80% <96.44%> (?)
agent-for-php-7.4 78.50% <96.44%> (?)
agent-for-php-8.0 78.52% <96.46%> (?)
agent-for-php-8.1 78.50% <96.46%> (?)
agent-for-php-8.2 78.10% <96.46%> (+0.15%) ⬆️
agent-for-php-8.3 78.10% <96.46%> (?)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -62,6 +62,7 @@
"nr.syntheticsJobId": "jjjjjjj-jjjj-1234-jjjj-jjjjjjjjjjjj",
"nr.syntheticsMonitorId": "mmmmmmm-mmmm-1234-mmmm-mmmmmmmmmmmm",
"externalDuration": "??",
"externalCallCount": 1,
Copy link
Member

Choose a reason for hiding this comment

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

Where does this extra attribute come from?

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.

5 participants