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.

Sign up for GitHub

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

Jump to bottom

Use template class for in-memory data. #1496

Merged
merged 9 commits into from
Jul 18, 2022
Merged

Use template class for in-memory data. #1496

merged 9 commits into from
Jul 18, 2022

Conversation

yxue
Copy link
Contributor

@yxue yxue commented Jul 14, 2022

Fixes #1493

Changes

Please provide a brief description of the changes here.

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

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@yxue yxue requested a review from a team July 14, 2022 05:39
@codecov
Copy link

codecov bot commented Jul 14, 2022 •
edited
Loading

Codecov Report

Merging #1496 (d2a5631) into main (fba16c6) will decrease coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1496      +/-   ##
==========================================
- Coverage   84.73%   84.66%   -0.07%     
==========================================
  Files         155      156       +1     
  Lines        4787     4784       -3     
==========================================
- Hits         4056     4050       -6     
- Misses        731      734       +3
Impacted Files Coverage Δ
...de/opentelemetry/exporters/memory/in_memory_data.h 100.00% <100.00%> (ø)
...entelemetry/exporters/memory/in_memory_span_data.h 100.00% <100.00%> (ø)
...lemetry/exporters/memory/in_memory_span_exporter.h 90.91% <100.00%> (-0.39%) ⬇️
ext/src/http/client/curl/http_client_curl.cc 79.93% <0.00%> (-1.51%) ⬇️
sdk/src/trace/batch_span_processor.cc 91.41% <0.00%> (+0.79%) ⬆️

lalitb
lalitb approved these changes Jul 15, 2022
View reviewed changes
Copy link
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

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

LGTM

@yxue yxue force-pushed the in-memory-data branch from bfe7f7f to 3b7b323 Compare July 15, 2022 21:11
@yxue yxue force-pushed the in-memory-data branch from 3b7b323 to 655f0a8 Compare July 17, 2022 03:01
@yxue yxue force-pushed the in-memory-data branch from 655f0a8 to 74791eb Compare July 17, 2022 03:08
owent
owent approved these changes Jul 17, 2022
View reviewed changes
Copy link
Member

@owent owent left a comment

Choose a reason for hiding this comment

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

LGTM

marcalff
marcalff approved these changes Jul 17, 2022
View reviewed changes
Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fixes.

@lalitb lalitb merged commit 4062237 into open-telemetry:main Jul 18, 2022
@marcalff marcalff mentioned this pull request Aug 2, 2022
Closed
6 tasks
yxue added a commit to yxue/opentelemetry-cpp that referenced this pull request Dec 5, 2022
@yxue
Use template class for in-memory data. (open-telemetry#1496)
b3ed81d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lalitb lalitb lalitb approved these changes

@marcalff marcalff marcalff approved these changes

@owent owent owent approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace the in-memory span data with the templated in-memory data class
4 participants
@yxue @lalitb @owent @marcalff