Skip to content

Resource attributes fix#1720

Merged
cijothomas merged 4 commits into
open-telemetry:masterfrom
Austin-Tan:ResourceAttributesFix
Jan 26, 2021
Merged

Resource attributes fix#1720
cijothomas merged 4 commits into
open-telemetry:masterfrom
Austin-Tan:ResourceAttributesFix

Conversation

@Austin-Tan

@Austin-Tan Austin-Tan commented Jan 25, 2021

Copy link
Copy Markdown
Member

I messed up with Git commands on PR #1647 and accidentally added a whole host of various changes that I could not undo. The work has been recovered here but for discussion, please refer to the conversation in that request.

This is my first time working on a change that would require a rework of a unit test. Please let me know if I have done that incorrectly.

Fixes #1606 .

Changes

Resource Attributes fixed to allow the values of key-value pairs to be of any object type, matching the behavior of Activity's SetTag method.

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Design discussion issue #
  • Changes in public API reviewed

@Austin-Tan Austin-Tan requested a review from a team January 25, 2021 09:48
@codecov

codecov Bot commented Jan 25, 2021

Copy link
Copy Markdown

Codecov Report

Merging #1720 (462bd36) into master (17dd4a7) will decrease coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1720      +/-   ##
==========================================
- Coverage   82.18%   82.10%   -0.09%     
==========================================
  Files         250      250              
  Lines        6764     6767       +3     
==========================================
- Hits         5559     5556       -3     
- Misses       1205     1211       +6     
Impacted Files Coverage Δ
src/OpenTelemetry/Resources/Resource.cs 93.93% <100.00%> (+0.60%) ⬆️
...us/Implementation/PrometheusExporterEventSource.cs 63.63% <0.00%> (-9.10%) ⬇️
...ZPages/Implementation/ZPagesExporterEventSource.cs 56.25% <0.00%> (-6.25%) ⬇️
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs 76.47% <0.00%> (-5.89%) ⬇️
...ter.ZPages/Implementation/ZPagesActivityTracker.cs 97.14% <0.00%> (-2.86%) ⬇️
...Telemetry/Internal/SelfDiagnosticsEventListener.cs 92.85% <0.00%> (-1.43%) ⬇️

}

return false;
throw new System.ArgumentException("Attribute value is null", keyName);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we need to have a follow up PR where we should throw if key is null as well.

@cijothomas cijothomas merged commit 842daff into open-telemetry:master Jan 26, 2021
@Austin-Tan Austin-Tan deleted the ResourceAttributesFix branch January 28, 2021 22:58
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.

ResourceBuilder Attribute Value is lost when using int value

3 participants