Use dynatrace-metric-utils #9295
Conversation
|
Thanks so much for the pull request! |
1 similar comment
|
Thanks so much for the pull request! |
|
I know my company has signed the corporate CLA because my coworker @thschue has contributed here, but my manager is out for an Austrian holiday today so I might have to wait for tomorrow to figure out how to get my account authorized on that CLA. Unless someone else can help me determine how to convince the CLA check that I'm ok? |
|
Thanks so much for the pull request! |
Looks like new artifacts were built from this PR. Get them here!Artifact URLs |
|
This is the individual agreement. I believe my company already signed a company agreement. Do i still need to do the individual one? |
|
From looking at our CLA stuff on the website here I think you still need to sign an individual one. But may also be worth checking with your manager to get your details on the company one. |
|
Yeah I'll check with him tomorrow as Austria is on holiday today. |
helenosheaa
added
the
feat
|
He's looking into getting me added to the corporate CLA, but this PR is ready for review in any case. It has already been through our internal review process and we have a couple of other changes that are in our internal review pipeline which will depend on it. |
|
!signed-cla |
|
Readme updated and CLA signed. Would appreciate a review when it is convenient because I have several follow-ups that will depend on this PR. |
Looks like new artifacts were built from this PR. Get them here!Artifact URLs |
|
Would really appreciate reviews on this. @ssoroka looks like you reviewed the last Dynatrace PR so your input would be appreciated. |
There was a problem hiding this comment.
Thanks for the PR. Few comments in the code.
My main question is whether this changes the schema of the metric ie will this break backwards compatibility for users. Would it be worth putting this change under a metric_version = 2 so that people can default to the previous behavior?
| url = "https://{your-environment-id}.live.dynatrace.com/api/v2/metrics/ingest" | ||
|
|
||
| ## API token is required if a URL is specified and should be restricted to the 'Ingest metrics' scope | ||
| api_token = "your API token here" // hard-coded for illustration only, should be read from environment |
There was a problem hiding this comment.
Would this make more sense api_token = "$API_TOKEN"
There was a problem hiding this comment.
I can change that. we didn't want to imply that the api token was automatically included in the environment or something
There was a problem hiding this comment.
either that or leave it as an empty string with a comment above
I wouldn't put it behind a version 2 because the format is the same and the normalization rules aren't different, but more corner cases are handled which will be able to handle a larger variety of names and prevent them from being dropped by our backend. |
|
The CI is telling me to run |
|
Make sure you're on the latest Go version when you run |
Ok I just wanted to be sure on the above. I'll try and get you another review from someone else this week so we can move forward with the PR. |
Even on the latest go (1.16.5) I am still getting no changes on go mod tidy |
|
Same here, I tried checking out this branch and running |
Looks like new artifacts were built from this PR. Get them here!Artifact URLs |
|
Deleting and recreating the go.sum seems to have worked :) |
(cherry picked from commit 298670a)
Dynatrace Metrics Exporter: Use the metric utils library maintained by Dynatrace Open Source to normalize and serialize metrics for the Dynatrace metrics ingest v2 endpoint.