Skip to content

Conversation

@JoeCqupt
Copy link

when i use BatchLogRecordProcessor/BatchSpanProcessor.
i want to know dropped ratio and exported success ratio and exported fail ratio and etc. but exist metrics can not compute those.

i want achieve this goal by updating processedSpans&processedLogs labels .

dropped ratio: dropped/processed
exported success ratio: exported/(processed - dropped)
exported fail ratio: 1 - exported/(processed - dropped)

@JoeCqupt JoeCqupt requested a review from a team July 17, 2023 05:39
@JoeCqupt JoeCqupt changed the title update processedSpans&processedLogs labels Change processedSpans&processedLogs labels Jul 17, 2023
@JoeCqupt JoeCqupt force-pushed the main branch 2 times, most recently from b006926 to d9461d9 Compare July 17, 2023 06:53
@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01 ⚠️

Comparison is base (b0c5c04) 91.32% compared to head (2749de9) 91.31%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5631      +/-   ##
============================================
- Coverage     91.32%   91.31%   -0.01%     
+ Complexity     4979     4978       -1     
============================================
  Files           554      554              
  Lines         14731    14739       +8     
  Branches       1376     1376              
============================================
+ Hits          13453    13459       +6     
  Misses          884      884              
- Partials        394      396       +2     
Impacted Files Coverage Δ
...metry/sdk/logs/export/BatchLogRecordProcessor.java 91.03% <100.00%> (-0.46%) ⬇️
...telemetry/sdk/trace/export/BatchSpanProcessor.java 94.63% <100.00%> (+0.14%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@JoeCqupt
Copy link
Author

@jkwatson @jack-berg PTAL

@jkwatson
Copy link
Contributor

This removes an existing attribute, correct? I'm worried it will break existing dashboards if we get rid of an attribute. @jack-berg is there a spec for these metrics yet?

@JoeCqupt
Copy link
Author

this modification is incompatible。 If you think compatibility is necessary, I can keep the original label and then add new labels

AttributeKey.stringKey("logRecordProcessorType");
private static final AttributeKey<Boolean> LOG_RECORD_PROCESSOR_DROPPED_LABEL =
AttributeKey.booleanKey("dropped");
private static final AttributeKey<String> LOG_RECORD_PROCESS_STATUS_LABEL =
Copy link
Contributor

Choose a reason for hiding this comment

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

Strongly recommend avoiding using the term "label" to refer to OpenTelemetry attributes. Let's try and be consistent in our naming. 🙏🏻

@jack-berg
Copy link
Member

This is being actively discussed in open-telemetry/semantic-conventions#184. In order to reduce churn, I'm inclined to hold any changes to these metrics until a new set of experimental semantic conventions lands.

@JoeCqupt would you be able to comment over on that PR with your use case / requirements?

@JoeCqupt JoeCqupt closed this Nov 20, 2023
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.

4 participants