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

update NumberDataPoint to support DoubleVal and IntVal #3689

Merged

Conversation

codeboten
Copy link
Contributor

Description: updating NumberDataPoint to support both Double and Int, this is a stepping stone to remove IntDataPoint altogether.

Link to tracking Issue: Part of #3534

@project-bot project-bot bot added this to In progress in Collector Jul 21, 2021
model/pdata/generated_metrics.go Outdated Show resolved Hide resolved
model/pdata/metrics.go Outdated Show resolved Hide resolved
cmd/pdatagen/internal/base_fields.go Outdated Show resolved Hide resolved
cmd/pdatagen/internal/base_fields.go Outdated Show resolved Hide resolved
cmd/pdatagen/internal/base_fields.go Show resolved Hide resolved
@codeboten codeboten force-pushed the codeboten/add-int-double-datapoint branch from 4274350 to 096ba31 Compare July 21, 2021 22:27
@codeboten codeboten force-pushed the codeboten/add-int-double-datapoint branch from 096ba31 to a7fcf19 Compare July 21, 2021 22:56
cmd/pdatagen/internal/base_fields.go Outdated Show resolved Hide resolved
cmd/pdatagen/internal/base_fields.go Outdated Show resolved Hide resolved
cmd/pdatagen/internal/base_fields.go Outdated Show resolved Hide resolved
@codeboten codeboten force-pushed the codeboten/add-int-double-datapoint branch from a7fcf19 to e7f06ca Compare July 21, 2021 23:27
@codeboten codeboten marked this pull request as ready for review July 21, 2021 23:28
@codeboten codeboten requested review from alolita, Aneurysm9 and a team as code owners July 21, 2021 23:28
Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

LGTM only 2 minor comments

Comment on lines 2017 to 2018
tv.SetDoubleVal(float64(17.13))
tv.SetIntVal(int64(17))
Copy link
Member

Choose a reason for hiding this comment

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

Should this be only one set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SetDoubleVal for now, added a TODO to remember to test with both int and double values in the future.

model/pdata/metrics.go Outdated Show resolved Hide resolved
@codeboten codeboten force-pushed the codeboten/add-int-double-datapoint branch from e7f06ca to e1058b2 Compare July 22, 2021 19:45
Collector automation moved this from In progress to Reviewer approved Jul 22, 2021
@bogdandrutu bogdandrutu merged commit 1ede290 into open-telemetry:main Jul 22, 2021
Collector automation moved this from Reviewer approved to Done Jul 22, 2021
@codeboten codeboten deleted the codeboten/add-int-double-datapoint branch June 22, 2023 15:17
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
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants