Support for Dynatrace V2 API #2619

jonatan-ivanov · 2021-05-26T01:52:51Z

* Move Dynatrace metrics exporter for API v1 to a separate package * rename dynatrace exporter * use an enum for Dynatrace API versions. * make DynatraceNamingConvention public * fix formatting * rename classes to have version at the end, move enum to uppercase, remove thread factory from exporter * move naming convention and delegate to the v1 implementation * update config validation and tests * rename variables in test files * remove unused imports * split out validation * re-add the apiVersion check and test * update misleading comment and remove duplicate apiVersion check

Unfortunately Java 8 does not support private methods in interfaces (9+ does) and I'm not sure we should have a public validateV1 method

- javadoc - ctor/method reordering (to be consistent and ease diffing) - get the logger based on Class not by String - start the threadpool at the last step in the ctor - license - static imports

…raceExporterV1

# Conflicts: # implementations/micrometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/DynatraceNamingConvention.java # implementations/micrometer-registry-dynatrace/src/test/java/io/micrometer/dynatrace/v1/DynatraceNamingConventionV1Test.java

Co-authored-by: Armin Ruech <[email protected]>

…e-oss-contrib/micrometer into dynatrace-oss-contrib-add-dynatrace-registry-v2 # Conflicts: # implementations/micrometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/AbstractDynatraceExporter.java # implementations/micrometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/DynatraceApiVersion.java # implementations/micrometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/DynatraceConfig.java # implementations/micrometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/DynatraceMeterRegistry.java # implementations/micrometer-registry-dynatrace/src/main/java/io/micrometer/dynatrace/v1/DynatraceExporterV1.java # implementations/micrometer-registry-dynatrace/src/test/java/io/micrometer/dynatrace/DynatraceConfigTest.java

(Parenthesis is not a must in Groovy)

…gistry

jonatan-ivanov · 2021-05-26T03:55:08Z

@pirgeo I tried to improve this a little, I haven't checked the validation logic and the tests (including the Nan/Inf behavior) but I'm going to do it soon.

shank9918 · 2021-05-27T08:11:36Z

Hi @jonatan-ivanov, just wanted to check on the availability of these changes. We are using Dynatrace V1 APIs with micrometer-registry-dynatrace at the moment and looking forward to using the V2 APIs. So, when do you think these will changes will be released?

jonatan-ivanov · 2021-05-27T22:47:58Z

@shank9918 After this PR get merged, you can use it as a snapshot release (should happen soon, see my previous comment about what needs to be done).
As for non-snapshot releases, we need sync-up with the Dynatrace OSS Team and figure out how/when to release this.

(cherry picked from commit 46f2b77b747b017ba37a0ef585b98d0dc65c3c02)

… to be sent (cherry picked from commit 2c43bddc5e260d44dbc0f8f0b479e08fd18c3771) Co-authored-by: Georg Pirklbauer <[email protected]>

...ter-registry-dynatrace/src/test/java/io/micrometer/dynatrace/v2/DynatraceExporterV2Test.java

- Using MockClock instead of Thread.sleep - Improve test setup - Fix/improve tests for: Gauge, TimeGauge, Counter, FunctionCounter, Timer, FunctionTimer, LongTaskTimer, DistributionSummary

- Custom Meter - Invalid cases (also dedupe)

…ttp client

jonatan-ivanov · 2021-06-03T18:57:54Z

@pirgeo I think this is close to be merged, I fixed a couple of things:

Added your proposal for the validation
Fixed tests by using MockClock instead of Thread.sleep
Registering 0th percentile (for min) overwrote the user-defined ones (if any) so I implemented a fix

I also have a few notes, just to double check if these are intended and you are fine with them:

Registering 0th percentile (for min) will also mean that the 0th percentile gauge will be reported to Dynatrace, this happens if I register a timer without any user-defined tags or percentiles

test-timer-v2,dt.metrics.source=micrometer gauge,min=125.82912,max=190.0,sum=954.0,count=6 1622746380008
test-timer-v2.percentile,phi=0,dt.metrics.source=micrometer gauge,125.82912 1622746380011

The Dynatrace java lib does not filter out empty tags, so this will be sent on the wire:

test-timer-v2,tag1=value1,dt.metrics.source=micrometer,tag2= gauge,min=142.606336,max=191.0,sum=866.0,count=5 1622745540019

Please notice tag2=. This does not seem to cause any issues, the tag is filtered out on the server-side.

Could you please check if these look ok for you?

pirgeo

@jonatan-ivanov Thanks, the changes are looking very good to me.

Regarding the 0th percentile export: We would rather not export the extra percentile line. Do you have any suggestions as to how we could drop just that line, preferably without having to look through the created lines and dropping it after creation?
Regarding the empty tags, this is something we would like to keep, for now. As you saw, empty tags are currently dropped upon ingestion, but might be used in the future. I have added suggestions to change the test method names.

...ter-registry-dynatrace/src/test/java/io/micrometer/dynatrace/v2/DynatraceExporterV2Test.java

Co-authored-by: Georg Pirklbauer <[email protected]>

jonatan-ivanov · 2021-06-08T02:41:44Z

@pirgeo Great, I'm happy you liked them.

Empty tags: Makes sense, thanks for catching the test names, I've merged your changes in.

0th percentile export: I agree. I think we should not export the extra percentile line, I wasn't 100% sure if it was intentional or not. I think there are two ways to solve this:

Hardcoding a 0 value if there is no good use-case for min (we are doing this for other meter registries)
Using the same MeterFilter to deny the artificially created 0th percentile Meters. This is somewhat tricky/hacky since accept does not know about the DistributionConfig. Because of this, we need to record which meters can be the subject of removal in the config phase, see: 21da805.
This fortunately happens once per registered meter and before the registry or the exporter would receive it. Also, the exporter does not need the Meter itself, it only needs the HistogramSnapshot, please see the tests.

What do you think?

jonatan-ivanov · 2021-06-08T18:17:42Z

I merged it based on the email exchange we had about this.
@pirgeo Thank you very much for your epic contribution, we really appreciate your help!

pirgeo · 2021-06-09T07:37:08Z

@jonatan-ivanov Thanks a lot! Appreciate your commitment and your help :)

pirgeo and others added 22 commits April 20, 2021 15:46

Adding validation logic into validate in DynatraceConfig

4f53492

Unfortunately Java 8 does not support private methods in interfaces (9+ does) and I'm not sure we should have a public validateV1 method

Polishing Dynatrace V1 refactor

5213b53

- javadoc - ctor/method reordering (to be consistent and ease diffing) - get the logger based on Class not by String - start the threadpool at the last step in the ctor - license - static imports

Breaking circular dependency between DynatraceMeterRegistry and Dynat…

1115208

…raceExporterV1

Add an exporter for the Dynatrace metrics API v2

8965981

Co-authored-by: Armin Ruech <[email protected]>

remove a comma that might lead to invalid ingestion result parsing

1845333

Address code review comments

54fd167

Use new library features to make the exporter code more concise

bcd543d

Merge branch 'main' into add-dynatrace-registry-v2

42b90c0

Making dependency configuration calls consistent

90efd94

(Parenthesis is not a must in Groovy)

Clean-up comments in DynatraceConfig (no validation changes)

ed896c6

Fixing license header in DynatraceExporterV2

d68f179

Converting config field to local variable in ctor of DynatraceMeterRe…

2781781

…gistry

DynatraceExporterV2 refactor: renamings, method order/visibility

bc2f34a

Simplify DynatraceExporterV2

4414474

Adding back partitioning for Dynatrace V1

439381f

More formatting for DynatraceExporterV2

be72004

Simplifying toMeterLine and createMeterLine methods

c1f1464

Improving naming and error messages

23f7f73

Fixing tests after refactor

3469809

jonatan-ivanov and others added 3 commits May 27, 2021 15:50

Merge branch 'main' into add-dynatrace-registry-v2

72a93b3

Implement fallback for config uri.

387a54f

(cherry picked from commit 46f2b77b747b017ba37a0ef585b98d0dc65c3c02)

add a warning on unlikely configurations and check if the token needs…

168c25f

… to be sent (cherry picked from commit 2c43bddc5e260d44dbc0f8f0b479e08fd18c3771) Co-authored-by: Georg Pirklbauer <[email protected]>

jonatan-ivanov commented Jun 2, 2021

View reviewed changes

...ter-registry-dynatrace/src/test/java/io/micrometer/dynatrace/v2/DynatraceExporterV2Test.java Outdated Show resolved Hide resolved

jonatan-ivanov marked this pull request as ready for review June 2, 2021 17:59

jonatan-ivanov added 6 commits June 2, 2021 15:02

Fix/improve DynatraceExporterV2Test, round one:

800519b

- Using MockClock instead of Thread.sleep - Improve test setup - Fix/improve tests for: Gauge, TimeGauge, Counter, FunctionCounter, Timer, FunctionTimer, LongTaskTimer, DistributionSummary

Fix/improve DynatraceExporterV2Test, round two:

5bfdd25

- Custom Meter - Invalid cases (also dedupe)

DynatraceExporterV2Test: static imports, nullable problems

395e087

Initialize DynatraceExporterV2Test the JUnit way

06fd508

Improving DynatraceExporterV2Test and test the interaction with the h…

81cf135

…ttp client

Fixing overwriting user-defined percentiles.

499a4d8

jonatan-ivanov added 3 commits June 3, 2021 12:59

re-enabling empty tag tests

d69d75e

Adding DynatraceMeterRegistryTest

a1d76fa

+javadoc

f4a488d

pirgeo reviewed Jun 7, 2021

View reviewed changes

...ter-registry-dynatrace/src/test/java/io/micrometer/dynatrace/v2/DynatraceExporterV2Test.java Outdated Show resolved Hide resolved

...ter-registry-dynatrace/src/test/java/io/micrometer/dynatrace/v2/DynatraceExporterV2Test.java Outdated Show resolved Hide resolved

jonatan-ivanov and others added 3 commits June 7, 2021 13:54

Rename blank tag tests, part one

a2571c3

Co-authored-by: Georg Pirklbauer <[email protected]>

Rename blank tag tests, part two

fe48423

Co-authored-by: Georg Pirklbauer <[email protected]>

Removing artificially added 0th percentile meters

21da805

Merge branch 'main' into add-dynatrace-registry-v2

688fa6c

jonatan-ivanov merged commit 258a2f8 into main Jun 8, 2021

jonatan-ivanov deleted the add-dynatrace-registry-v2 branch June 8, 2021 18:12

jonatan-ivanov linked an issue Jun 8, 2021 that may be closed by this pull request

Support for Dynatrace metrics API v2 #2295

Closed

pirgeo mentioned this pull request Feb 19, 2024

Zero percentile can be missing from Dynatrace meters where expected #4750

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support for Dynatrace V2 API #2619

Support for Dynatrace V2 API #2619

Uh oh!

jonatan-ivanov commented May 26, 2021 •

edited

Loading

Uh oh!

jonatan-ivanov commented May 26, 2021

Uh oh!

shank9918 commented May 27, 2021 •

edited

Loading

Uh oh!

jonatan-ivanov commented May 27, 2021

Uh oh!

Uh oh!

jonatan-ivanov commented Jun 3, 2021 •

edited

Loading

Uh oh!

pirgeo left a comment

Uh oh!

Uh oh!

Uh oh!

jonatan-ivanov commented Jun 8, 2021

Uh oh!

jonatan-ivanov commented Jun 8, 2021

Uh oh!

pirgeo commented Jun 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Support for Dynatrace V2 API #2619

Support for Dynatrace V2 API #2619

Uh oh!

Conversation

jonatan-ivanov commented May 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonatan-ivanov commented May 26, 2021

Uh oh!

shank9918 commented May 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonatan-ivanov commented May 27, 2021

Uh oh!

Uh oh!

jonatan-ivanov commented Jun 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pirgeo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jonatan-ivanov commented Jun 8, 2021

Uh oh!

jonatan-ivanov commented Jun 8, 2021

Uh oh!

pirgeo commented Jun 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jonatan-ivanov commented May 26, 2021 •

edited

Loading

shank9918 commented May 27, 2021 •

edited

Loading

jonatan-ivanov commented Jun 3, 2021 •

edited

Loading