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

Rename pdata.AttributeValue to pdata.Value #4978

Merged
merged 2 commits into from
Mar 16, 2022

Conversation

dmitryax
Copy link
Member

@dmitryax dmitryax commented Mar 10, 2022

pdata.AttributeValue is a wrapper over opentelemetry.proto.common.v1.AnyValue type. It's not exclusively used with Attributes field, pdata.LogRecord.Body also uses it which makes the name misleading. This change renames all API related to pdata.AttributeValue to pdata.Value keeping the old API as deprecated.

Updates: #4818

UPD: The PR is updated to use Value naming instead of AnyValue as suggested by @bogdandrutu in #4988 (comment)

@codecov
Copy link

codecov bot commented Mar 10, 2022

Codecov Report

Merging #4978 (c09bb3b) into main (e214f71) will increase coverage by 0.03%.
The diff coverage is 97.50%.

@@            Coverage Diff             @@
##             main    #4978      +/-   ##
==========================================
+ Coverage   90.92%   90.96%   +0.03%     
==========================================
  Files         183      183              
  Lines       10692    10692              
==========================================
+ Hits         9722     9726       +4     
+ Misses        752      749       -3     
+ Partials      218      217       -1     
Impacted Files Coverage Δ
model/internal/pdata/common.go 95.33% <97.01%> (+0.88%) ⬆️
internal/otlptext/databuffer.go 100.00% <100.00%> (ø)
model/internal/pdata/generated_common.go 100.00% <100.00%> (ø)
model/internal/pdata/generated_log.go 96.65% <100.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 e214f71...c09bb3b. Read the comment docs.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Overall the change looks good, just some comments that need updating.

CHANGELOG.md Outdated Show resolved Hide resolved
model/internal/pdata/common.go Outdated Show resolved Hide resolved
model/internal/pdata/common.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@dmitryax
Copy link
Member Author

@codeboten thanks for the review. I addressed all your comments

@dmitryax dmitryax force-pushed the pdata-any-value branch 2 times, most recently from 555da20 to 11dc60a Compare March 11, 2022 19:10
@dmitryax dmitryax marked this pull request as draft March 12, 2022 18:24
@dmitryax dmitryax force-pushed the pdata-any-value branch 4 times, most recently from ff2e1e5 to 081b8b9 Compare March 13, 2022 23:53
@dmitryax
Copy link
Member Author

dmitryax commented Mar 13, 2022

I realized that this PR solves only part of the problem.

Removed AttributeValueSlice renaming from this PR for now and submitted another follow-up issue: #4988

Will keep this PR in draft until we have an agreement on the path forward.

@dmitryax dmitryax changed the title Rename pdata.AttributeValue to pdata.AnyValue Rename pdata.AttributeValue to pdata.Value Mar 15, 2022
@dmitryax dmitryax requested a review from codeboten March 15, 2022 05:22
@dmitryax dmitryax marked this pull request as ready for review March 15, 2022 05:26
@dmitryax
Copy link
Member Author

The PR is updated to use Value naming instead of AnyValue as suggested by @bogdandrutu in #4988 (comment)

@dmitryax dmitryax force-pushed the pdata-any-value branch 3 times, most recently from bc6dee5 to 06b6862 Compare March 15, 2022 19:16
`pdata.AttributeValue` is a wrapper over `opentelemetry.proto.common.v1.AnyValue` type. It's not exclusively used with Attributes field, `pdata.LogRecord.Body` also uses it which makes the name misleading. This change renames all API related to `pdata.AttributeValue` to `pdata.Value` keeping the old API as deprecated
@codeboten codeboten merged commit 89115dd into open-telemetry:main Mar 16, 2022
@dmitryax dmitryax deleted the pdata-any-value branch March 16, 2022 16:43
Nicholaswang pushed a commit to Nicholaswang/opentelemetry-collector that referenced this pull request Jun 7, 2022
`pdata.AttributeValue` is a wrapper over `opentelemetry.proto.common.v1.AnyValue` type. It's not exclusively used with Attributes field, `pdata.LogRecord.Body` also uses it which makes the name misleading. This change renames all API related to `pdata.AttributeValue` to `pdata.Value` keeping the old API as deprecated

Co-authored-by: Bogdan Drutu <[email protected]>
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.

3 participants