Add resettable DistributionSummary and Timer for Dynatrace registry #3093
Conversation
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/DynatraceMeterRegistry.java
Outdated
Show resolved
Hide resolved
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/DynatraceMeterRegistry.java
Outdated
Show resolved
Hide resolved
...stry-dynatrace/src/main/java/io/micrometer/dynatrace/types/DynatraceDistributionSummary.java
Show resolved
Hide resolved
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/types/DynatraceSummary.java
Outdated
Show resolved
Hide resolved
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/types/DynatraceSummary.java
Outdated
Show resolved
Hide resolved
...icrometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/types/DynatraceTimer.java
Outdated
Show resolved
Hide resolved
...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
Show resolved
Hide resolved
pirgeo
force-pushed
the
add-resettable-summaries
branch
from
22e1b38
to
8b07d28
Compare
There was a problem hiding this comment.
Thanks for the input, @jonatan-ivanov!
I updated the PR with a bunch of changes based on your comments. Originally, I did not base the DynatraceTimer and DynatraceDistributionSummary off of AbstractTimer and AbstractDistributionSummary since those already track a Histogram, which we currently do not need. Since the underlying data structure is rather simple (only min/max/sum/count) we do not necessarily want to track the whole histogram. I must have missed the NoopHistogram in my first pass, thats why I implemented the interfaces directly. By basing our instruments off the Abstract instruments, the implementation actually became simpler. I set the Dynatrace instruments up in a way where the Histogram will always be a NoopHistogram.
I also added a toggle to the DynatraceConfig now that allows for switching back to the original Micrometer-provided instruments, and rebased it on 1.9.0.
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/DynatraceMeterRegistry.java
Outdated
Show resolved
Hide resolved
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/DynatraceMeterRegistry.java
Outdated
Show resolved
Hide resolved
...stry-dynatrace/src/main/java/io/micrometer/dynatrace/types/DynatraceDistributionSummary.java
Show resolved
Hide resolved
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/types/DynatraceSummary.java
Outdated
Show resolved
Hide resolved
...rometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/types/DynatraceSummary.java
Outdated
Show resolved
Hide resolved
...icrometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/types/DynatraceTimer.java
Outdated
Show resolved
Hide resolved
...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
Show resolved
Hide resolved
| return; | ||
| } | ||
|
|
||
| synchronized (this) { |
There was a problem hiding this comment.
I'm thinking if we can get into trouble because of this or using a ReadWriteLock and non concurrent types would be more performant. Since Atomic* and *Adder classes are lock free, I'm not sure.
jonatan-ivanov
added
enhancement
|
@pirgeo Thank you very much! |
related to #3007
This PR introduces resettable
TimerandDistributionSummaryinstruments to be used in the Dynatrace v2 exporter to ensure no max-buffering from previous export intervals ends up in the exported lines. These types also add support for tracking the minimum since the last export.