Skip to content
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

serialize each datapoint into separate line #2618

Merged
merged 3 commits into from
Mar 15, 2021

Conversation

sahejsingh
Copy link
Contributor

Description: Dynatrace API api expects each metric and its value to be on a separate line.

I might be wrong here, but to me it looks like the Dynatrace exporter incorrectly concatenates multiple data points for a single metric into one line. When this one line exceeds 2000 chars, the dynatrace server returns an error.

This change changes the way a DataPointSlice is serialized.

Now each data data point is serialised into its own line. The chunking later on can easily break this into 1000 lines (as required by the api).

versions:
otel library (used by client app): v0.17.0
otel-collector: 0.21.0
dynatrace: 1.211.111.20210222-093946

Link to tracking Issue: none.

Testing: unit testing and also able to push the metrics to a live instance of dynatrace server.

Documentation: none.

@sahejsingh sahejsingh requested a review from arminru as a code owner March 8, 2021 23:06
@sahejsingh sahejsingh requested a review from a team March 8, 2021 23:06
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 8, 2021

CLA Signed

The committers are authorized under a signed CLA.

  • ✅ Sahejpreet Singh (2e9c7c6aec213a5dd6fc8f3cc3e08be11546ec37, 7d6d4c8d9ed577a7e716c9bb296b86a8ee1b545a, ad559df2e141ea9d8c6c0f94fc9e4bf3dba85e9a)

@codecov
Copy link

codecov bot commented Mar 8, 2021

Codecov Report

Merging #2618 (e88c487) into main (047b1d9) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2618      +/-   ##
==========================================
+ Coverage   91.47%   91.48%   +0.01%     
==========================================
  Files         439      439              
  Lines       21855    21858       +3     
==========================================
+ Hits        19991    19996       +5     
+ Misses       1394     1392       -2     
  Partials      470      470              
Flag Coverage Δ
integration 69.18% <ø> (-0.07%) ⬇️
unit 90.40% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
exporter/dynatraceexporter/metrics_exporter.go 86.66% <100.00%> (+0.39%) ⬆️
...r/dynatraceexporter/serialization/serialization.go 100.00% <100.00%> (ø)
processor/groupbytraceprocessor/event.go 96.77% <0.00%> (+0.80%) ⬆️
receiver/carbonreceiver/transport/tcp_server.go 68.00% <0.00%> (+1.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 047b1d9...e88c487. Read the comment docs.

@arminru arminru requested a review from dyladan March 9, 2021 08:27
Copy link
Member

@arminru arminru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @sahejsingh!

@bogdandrutu
Copy link
Member

@arminru @sahejsingh I cannot merge this until CLA is signed sorry.

@sahejsingh
Copy link
Contributor Author

@bogdandrutu CLA signed 👍

@bogdandrutu
Copy link
Member

@sahejsingh now the most important part, fix the build :)) Most likely run make gotidy or just make to see the errors

@sahejsingh
Copy link
Contributor Author

ah, I should have paid more attention. Will fix it now.

@sahejsingh sahejsingh force-pushed the fix-dynatrace-exporter branch from ad559df to d521d5f Compare March 14, 2021 22:51
@sahejsingh
Copy link
Contributor Author

@bogdandrutu all checks have passed!

@bogdandrutu bogdandrutu merged commit 4dbba92 into open-telemetry:main Mar 15, 2021
@sahejsingh sahejsingh deleted the fix-dynatrace-exporter branch March 15, 2021 22:25
pmatyjasek-sumo referenced this pull request in pmatyjasek-sumo/opentelemetry-collector-contrib Apr 28, 2021
* split each datapoint into separate point

* fix unit tests

* change data in unit tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants