-
Notifications
You must be signed in to change notification settings - Fork 992
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
[Dynatrace v2] Update to utils library v2 #4064
[Dynatrace v2] Update to utils library v2 #4064
Conversation
Updated to dynatrace-metric-utils v2
99e12cc
to
ac0c96c
Compare
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/v2/DynatraceExporterV2.java
Show resolved
Hide resolved
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/v2/DynatraceExporterV2.java
Outdated
Show resolved
Hide resolved
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/v2/DynatraceExporterV2.java
Outdated
Show resolved
Hide resolved
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/v2/DynatraceExporterV2.java
Outdated
Show resolved
Hide resolved
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/v2/DynatraceExporterV2.java
Outdated
Show resolved
Hide resolved
@@ -493,4 +524,23 @@ private void storeMetadataLine(Metric.Builder metricBuilder, Map<String, String> | |||
} | |||
} | |||
|
|||
private String extractMetricKey(String metadataLine) { |
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.
Can't MetadataStep
do this for us instead of encoding it into a string and then trying to decode it again?
This also feels quite brittle. Btw should the index start at 1?
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.
Btw should the index start at 1?
Yes that's on purpose, the first index will always be #
, I added a clarifying comment in 0d6d7f0
…der before calling build() on it
…> enrichMetadataStep()
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.
Thank you for the review. 🙂
I addressed some comments while @pirgeo is out of office
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/v2/DynatraceExporterV2.java
Show resolved
Hide resolved
@@ -493,4 +524,23 @@ private void storeMetadataLine(Metric.Builder metricBuilder, Map<String, String> | |||
} | |||
} | |||
|
|||
private String extractMetricKey(String metadataLine) { |
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.
Btw should the index start at 1?
Yes that's on purpose, the first index will always be #
, I added a clarifying comment in 0d6d7f0
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/v2/DynatraceExporterV2.java
Outdated
Show resolved
Hide resolved
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/v2/DynatraceExporterV2.java
Outdated
Show resolved
Hide resolved
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/v2/DynatraceExporterV2.java
Outdated
Show resolved
Hide resolved
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/v2/DynatraceExporterV2.java
Outdated
Show resolved
Hide resolved
See gh-4064 Co-authored-by: Georg Pirklbauer <georg.pirklbauer@dynatrace.com> Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com> Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com>
I polished the PR a bit and merged it in, thank you all for the contribution. |
This PR updates the Dynatrace exporter v2 to use the new dynatrace-metric-utils library v2.0.0 which provides a big boost in performance while also reducing the memory overhead. v2 has been released on Maven central.