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

Use json for writing floats as strings #4934

Merged
merged 5 commits into from
Mar 17, 2022

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Mar 1, 2022

Description:

This is the change that would be required by open-telemetry/opentelemetry-specification#2343. It changes how small floats are represented by using the json marshaller. The test would fail with the current implementation, as it returns ".000000009" instead of "9e-9".

@codecov
Copy link

codecov bot commented Mar 1, 2022

Codecov Report

Merging #4934 (5d5dfa5) into main (f3fd237) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #4934      +/-   ##
==========================================
- Coverage   91.06%   91.04%   -0.03%     
==========================================
  Files         180      180              
  Lines       10680    10700      +20     
==========================================
+ Hits         9726     9742      +16     
- Misses        737      740       +3     
- Partials      217      218       +1     
Impacted Files Coverage Δ
model/internal/pdata/common.go 94.68% <100.00%> (-0.66%) ⬇️

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 f3fd237...5d5dfa5. Read the comment docs.

model/pdata/common.go Outdated Show resolved Hide resolved
@dashpole dashpole force-pushed the json_conversion branch 2 times, most recently from 7b87ae9 to 7e5de54 Compare March 3, 2022 14:23
@dashpole dashpole marked this pull request as ready for review March 3, 2022 14:23
@dashpole dashpole requested a review from a team as a code owner March 3, 2022 14:23
@dashpole dashpole force-pushed the json_conversion branch 4 times, most recently from c7d01d5 to fc5ac27 Compare March 9, 2022 16:31
@dashpole
Copy link
Contributor Author

dashpole commented Mar 9, 2022

This is ready for review

Collector automation moved this from In progress to Reviewer approved Mar 16, 2022
@bogdandrutu
Copy link
Member

@dashpole maybe rebase to re-trigger checks?

@dashpole
Copy link
Contributor Author

pushed again

@bogdandrutu bogdandrutu reopened this Mar 16, 2022
Collector automation moved this from Reviewer approved to In progress Mar 16, 2022
@bogdandrutu
Copy link
Member

@dashpole not sure what happens,.... maybe open a new PR from a cloned branch?

@dashpole
Copy link
Contributor Author

it woke up!

@dashpole
Copy link
Contributor Author

rebased

@bogdandrutu bogdandrutu merged commit 6250957 into open-telemetry:main Mar 17, 2022
@dashpole dashpole deleted the json_conversion branch March 17, 2022 13:23
@codeboten codeboten moved this from In progress to Reviewer approved in Collector Mar 28, 2022
Nicholaswang pushed a commit to Nicholaswang/opentelemetry-collector that referenced this pull request Jun 7, 2022
* use json for writing floats as strings

* avoid using reflection for floats

* extra unit test

* fix rebase

Co-authored-by: Bogdan Drutu <bogdandrutu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Collector
  
Reviewer approved
Development

Successfully merging this pull request may close these issues.

None yet

2 participants