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

allow extension of the lifetime of ContextStorage. #1214

Merged
merged 6 commits into from
Feb 19, 2022

Conversation

esigo
Copy link
Member

@esigo esigo commented Feb 15, 2022

Fixes #1211 (issue)

Changes

The specifications require the context to be immutable, but does not talk about whether the users are allowed to have access to the storage.
The change in this PR allows the users to get a shared_ptr to const so they can't do anything with the pointer but extending the lifetime of the storage.

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

@esigo esigo requested a review from a team February 15, 2022 17:36
@codecov
Copy link

codecov bot commented Feb 15, 2022

Codecov Report

Merging #1214 (bfbf9f4) into main (1026ec3) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1214   +/-   ##
=======================================
  Coverage   92.99%   92.99%           
=======================================
  Files         196      196           
  Lines        7038     7038           
=======================================
  Hits         6544     6544           
  Misses        494      494           
Impacted Files Coverage Δ
...pi/include/opentelemetry/context/runtime_context.h 97.50% <ø> (ø)

@ThomsonTan
Copy link
Contributor

Could we provide some dedicated helper method to increase the lifetime of RuntimeStorage? As the user cannot deduce the purpose of this API directly from the API name.

@esigo
Copy link
Member Author

esigo commented Feb 17, 2022

Could we provide some dedicated helper method to increase the lifetime of RuntimeStorage? As the user cannot deduce the purpose of this API directly from the API name.

Could you please elaborate?

@ThomsonTan
Copy link
Contributor

Could you please elaborate?

I meant some explicit API like AddRef/ReleaseRef but this seems requiring 2 APIs. The current one looks good to through.

@lalitb lalitb merged commit 3508d7c into open-telemetry:main Feb 19, 2022
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.

Expose static RuntimeContext storage for lifecycle management
3 participants