-
Notifications
You must be signed in to change notification settings - Fork 66
perf(azure-monitor-exporter): Azure monitor exporter log transformer optimizations #1731
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
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1731 +/- ##
==========================================
+ Coverage 84.06% 84.08% +0.02%
==========================================
Files 469 470 +1
Lines 136284 136558 +274
==========================================
+ Hits 114561 114828 +267
- Misses 21189 21196 +7
Partials 534 534
🚀 New features to boost your workflow:
|
drewrelmas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mention perf improvements - do you have benchmarks of before and after? I wonder if we should add these experimental components into benchmarks like the rest of primary components.
| impl LogRecordField { | ||
| /// Parse a field name string into a LogRecordField enum | ||
| fn from_str(s: &str) -> Option<Self> { | ||
| if s.eq_ignore_ascii_case("time_unix_nano") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker, but depending on how #1725 turns out, maybe we want to support aliases for the OpenTelemetry log data model spec naming convention. You're already partially flexible thanks to the case insensitivity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't bench it particularly, but I saw an about 3%-5% throughput increase in my local tests!
rust/otap-dataflow/crates/otap/src/experimental/azure_monitor_exporter/transformer.rs
Show resolved
Hide resolved
Nit - This is still there in readme and yaml. |
|
addressed the comments, ready to merge. |
Uh oh!
There was an error while loading. Please reload this page.