Skip to content

Remove ScopeBound... example implementations#819

Merged
brettmc merged 1 commit into
open-telemetry:mainfrom
Nevay:remove-scopebound-examples-classes
Sep 10, 2022
Merged

Remove ScopeBound... example implementations#819
brettmc merged 1 commit into
open-telemetry:mainfrom
Nevay:remove-scopebound-examples-classes

Conversation

@Nevay

@Nevay Nevay commented Sep 9, 2022

Copy link
Copy Markdown
Contributor

Removes ScopeBound... classes to reduce the API surface.

  • ScopeBoundCallable: Was intended for event loops. Event loop decorators should instead implement this in a way that fits the specific loop API the best with regard to avoiding unnecessary references to arguments and error handling.
    (The implementation was (mostly) sufficient for react/event-loop but shouldn't be used for revolt/event-loop.)
  • ScopeBoundPromise: Was only intended as very basic example implementation. An actual implementation has to hook into the specific promise implementation to capture the context of the initial promise.
    (I'm not sure how useful promise/future support is in general, there are scenarios where we cannot preserve the context association, e.g. when extracting/awaiting the value.)

@Nevay
Nevay requested a review from a team September 9, 2022 14:21
@codecov

codecov Bot commented Sep 9, 2022

Copy link
Copy Markdown

Codecov Report

Merging #819 (bf38311) into main (6e385b9) will decrease coverage by 0.07%.
The diff coverage is n/a.

❗ Current head bf38311 differs from pull request most recent head aab5c05. Consider uploading reports for the commit aab5c05 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #819      +/-   ##
============================================
- Coverage     82.88%   82.80%   -0.08%     
+ Complexity     1829     1803      -26     
============================================
  Files           225      222       -3     
  Lines          4697     4653      -44     
============================================
- Hits           3893     3853      -40     
+ Misses          804      800       -4     
Flag Coverage Δ
7.4 ?
8.0 ?
8.1 82.80% <ø> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/SDK/Metrics/Meter.php 99.08% <0.00%> (-0.92%) ⬇️
src/SDK/Common/Dsn/Factory.php 100.00% <0.00%> (ø)
src/SDK/Common/Util/WeakMap.php 0.00% <0.00%> (ø)
src/Context/ContextStorageNode.php 100.00% <0.00%> (ø)
src/Context/FiberBoundContextStorageScope.php 0.00% <0.00%> (ø)
src/Contrib/Context/Swoole/SwooleContextScope.php 0.00% <0.00%> (ø)
src/SDK/Common/Attribute/AttributesBuilder.php 84.84% <0.00%> (+1.51%) ⬆️

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 6e385b9...aab5c05. Read the comment docs.

Nevay added a commit to Nevay/opentelemetry-php that referenced this pull request Sep 9, 2022
Drop if open-telemetry#819 is merged first.
@brettmc
brettmc merged commit 776273c into open-telemetry:main Sep 10, 2022
@Nevay
Nevay deleted the remove-scopebound-examples-classes branch April 19, 2023 19:11
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.

2 participants