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

log: Introduce EnabledParameters #5791

Merged
merged 19 commits into from
Sep 13, 2024
Merged

log: Introduce EnabledParameters #5791

merged 19 commits into from
Sep 13, 2024

Conversation

pellared
Copy link
Member

@pellared pellared commented Sep 6, 2024 •
edited
Loading

Fixes #5769

Related spec PR: open-telemetry/opentelemetry-specification#4203

Remark: A follow-up in contrib is required and afterwards here to fix the example/dice.

Benchstat results for sdk/log (log has no benchmarks related to Enabled):

goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/log
cpu: Intel(R) Core(TM) i9-10885H CPU @ 2.40GHz
                                  │    old.txt    │               new.txt                │
                                  │    sec/op     │    sec/op     vs base                │
BatchProcessorOnEmit-16              398.8n ± 10%   395.6n ± 12%        ~ (p=0.971 n=10)
Processor/Simple-16                  882.2n ±  8%   869.8n ±  9%        ~ (p=0.811 n=10)
Processor/Batch-16                   1.478µ ±  3%   1.485µ ±  5%        ~ (p=0.646 n=10)
Processor/SetTimestampSimple-16      847.8n ±  1%   844.6n ±  3%        ~ (p=0.247 n=10)
Processor/SetTimestampBatch-16       1.480µ ±  3%   1.473µ ±  4%        ~ (p=0.700 n=10)
Processor/AddAttributesSimple-16     930.0n ±  1%   933.8n ±  1%        ~ (p=0.172 n=10)
Processor/AddAttributesBatch-16      1.624µ ±  2%   1.639µ ±  2%        ~ (p=0.839 n=10)
Processor/SetAttributesSimple-16     903.4n ±  1%   895.1n ±  1%        ~ (p=0.190 n=10)
Processor/SetAttributesBatch-16      1.554µ ±  4%   1.529µ ±  3%        ~ (p=0.159 n=10)
LoggerNewRecord/5_attributes-16      346.0n ±  2%   343.3n ±  2%        ~ (p=0.448 n=10)
LoggerNewRecord/10_attributes-16     1.608µ ±  6%   1.503µ ±  2%   -6.53% (p=0.007 n=10)
LoggerEnabled-16                    34.305n ±  8%   6.706n ±  1%  -80.45% (p=0.000 n=10)
LoggerProviderLogger-16              636.9n ± 10%   605.8n ±  3%        ~ (p=0.105 n=10)
WalkAttributes/1_attributes-16       5.363n ±  3%   4.540n ± 14%  -15.34% (p=0.002 n=10)
WalkAttributes/10_attributes-16      5.436n ±  7%   4.461n ±  2%  -17.95% (p=0.000 n=10)
WalkAttributes/100_attributes-16     5.126n ±  9%   4.465n ±  1%  -12.90% (p=0.000 n=10)
WalkAttributes/1000_attributes-16    5.316n ±  9%   4.502n ±  5%  -15.32% (p=0.002 n=10)
SetAddAttributes/SetAttributes-16    220.5n ± 18%   192.6n ± 11%  -12.67% (p=0.007 n=10)
SetAddAttributes/AddAttributes-16    165.3n ± 21%   127.3n ± 22%  -22.96% (p=0.011 n=10)
SimpleProcessorOnEmit-16             2.159n ±  9%   2.167n ±  9%        ~ (p=0.739 n=10)
geomean                              178.3n         154.5n        -13.31%

                        │    old.txt    │             new.txt             │
                        │      B/s      │      B/s       vs base          │
BatchProcessorOnEmit-16   76.52Mi ± 11%   77.14Mi ± 14%  ~ (p=0.971 n=10)

                                  │    old.txt     │                new.txt                │
                                  │      B/op      │     B/op      vs base                 │
BatchProcessorOnEmit-16               0.000 ± 0%       0.000 ± 0%       ~ (p=1.000 n=10) ¹
Processor/Simple-16                   417.0 ± 0%       417.0 ± 0%       ~ (p=1.000 n=10) ¹
Processor/Batch-16                  1.093Ki ± 1%     1.088Ki ± 1%       ~ (p=0.254 n=10)
Processor/SetTimestampSimple-16       417.0 ± 0%       417.0 ± 0%       ~ (p=1.000 n=10) ¹
Processor/SetTimestampBatch-16      1.095Ki ± 1%     1.084Ki ± 2%       ~ (p=0.361 n=10)
Processor/AddAttributesSimple-16      417.0 ± 0%       417.0 ± 0%       ~ (p=1.000 n=10) ¹
Processor/AddAttributesBatch-16     1.085Ki ± 1%     1.086Ki ± 1%       ~ (p=1.000 n=10)
Processor/SetAttributesSimple-16      465.0 ± 0%       465.0 ± 0%       ~ (p=1.000 n=10) ¹
Processor/SetAttributesBatch-16     1.129Ki ± 1%     1.125Ki ± 1%       ~ (p=0.084 n=10)
LoggerNewRecord/5_attributes-16       0.000 ± 0%       0.000 ± 0%       ~ (p=1.000 n=10) ¹
LoggerNewRecord/10_attributes-16      610.0 ± 0%       610.0 ± 0%       ~ (p=1.000 n=10) ¹
LoggerEnabled-16                      0.000 ± 0%       0.000 ± 0%       ~ (p=1.000 n=10) ¹
LoggerProviderLogger-16               359.0 ± 6%       346.0 ± 3%       ~ (p=0.117 n=10)
WalkAttributes/1_attributes-16        0.000 ± 0%       0.000 ± 0%       ~ (p=1.000 n=10) ¹
WalkAttributes/10_attributes-16       0.000 ± 0%       0.000 ± 0%       ~ (p=1.000 n=10) ¹
WalkAttributes/100_attributes-16      0.000 ± 0%       0.000 ± 0%       ~ (p=1.000 n=10) ¹
WalkAttributes/1000_attributes-16     0.000 ± 0%       0.000 ± 0%       ~ (p=1.000 n=10) ¹
SetAddAttributes/SetAttributes-16     48.00 ± 0%       48.00 ± 0%       ~ (p=1.000 n=10) ¹
SetAddAttributes/AddAttributes-16     0.000 ± 0%       0.000 ± 0%       ~ (p=1.000 n=10) ¹
SimpleProcessorOnEmit-16              0.000 ± 0%       0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                          ²                 -0.27%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                                  │   old.txt    │               new.txt               │
                                  │  allocs/op   │ allocs/op   vs base                 │
BatchProcessorOnEmit-16             0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Processor/Simple-16                 1.000 ± 0%     1.000 ± 0%       ~ (p=1.000 n=10) ¹
Processor/Batch-16                  1.000 ± 0%     1.000 ± 0%       ~ (p=1.000 n=10) ¹
Processor/SetTimestampSimple-16     1.000 ± 0%     1.000 ± 0%       ~ (p=1.000 n=10) ¹
Processor/SetTimestampBatch-16      1.000 ± 0%     1.000 ± 0%       ~ (p=1.000 n=10) ¹
Processor/AddAttributesSimple-16    1.000 ± 0%     1.000 ± 0%       ~ (p=1.000 n=10) ¹
Processor/AddAttributesBatch-16     1.000 ± 0%     1.000 ± 0%       ~ (p=1.000 n=10) ¹
Processor/SetAttributesSimple-16    2.000 ± 0%     2.000 ± 0%       ~ (p=1.000 n=10) ¹
Processor/SetAttributesBatch-16     2.000 ± 0%     2.000 ± 0%       ~ (p=1.000 n=10) ¹
LoggerNewRecord/5_attributes-16     0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
LoggerNewRecord/10_attributes-16    4.000 ± 0%     4.000 ± 0%       ~ (p=1.000 n=10) ¹
LoggerEnabled-16                    0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
LoggerProviderLogger-16             1.000 ± 0%     1.000 ± 0%       ~ (p=1.000 n=10) ¹
WalkAttributes/1_attributes-16      0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
WalkAttributes/10_attributes-16     0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
WalkAttributes/100_attributes-16    0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
WalkAttributes/1000_attributes-16   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
SetAddAttributes/SetAttributes-16   1.000 ± 0%     1.000 ± 0%       ~ (p=1.000 n=10) ¹
SetAddAttributes/AddAttributes-16   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
SimpleProcessorOnEmit-16            0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                        ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

@codecov Codecov
Copy link

codecov bot commented Sep 6, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.5%. Comparing base (a475ee2) to head (b4a7d02).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #5791     +/-   ##
=======================================
- Coverage   84.5%   84.5%   -0.1%     
=======================================
  Files        272     272             
  Lines      22766   22771      +5     
=======================================
  Hits       19251   19251             
- Misses      3172    3177      +5     
  Partials     343     343

see 2 files with indirect coverage changes

@pellared pellared mentioned this pull request Sep 6, 2024
Closed
@pellared pellared added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Sep 6, 2024
@pellared pellared self-assigned this Sep 6, 2024
@pellared pellared added pkg:API Related to an API package area:logs Part of OpenTelemetry logs labels Sep 6, 2024
@pellared pellared changed the title [Spike] log: Introduce EnabledParam log: Introduce EnabledParam Sep 9, 2024
@pellared pellared marked this pull request as ready for review September 9, 2024 09:05
dashpole
dashpole approved these changes Sep 9, 2024
View reviewed changes
log/enabled.go Outdated Show resolved Hide resolved
@pellared pellared marked this pull request as draft September 9, 2024 15:12
@pellared pellared removed the Skip Changelog PRs that do not require a CHANGELOG.md entry label Sep 9, 2024
@pellared pellared marked this pull request as ready for review September 9, 2024 15:23
@pellared pellared changed the title log: Introduce EnabledParam log: Introduce EnabledOpts Sep 9, 2024
MrAlias
MrAlias reviewed Sep 9, 2024
View reviewed changes
log/enabled.go Outdated Show resolved Hide resolved
dashpole
dashpole approved these changes Sep 10, 2024
View reviewed changes
example/dice/rolldice.go Outdated Show resolved Hide resolved
@pellared pellared mentioned this pull request Sep 10, 2024
Open
5 tasks
MrAlias
MrAlias reviewed Sep 11, 2024
View reviewed changes
log/enabled.go Outdated Show resolved Hide resolved
MrAlias
MrAlias reviewed Sep 11, 2024
View reviewed changes
log/enabled.go Outdated Show resolved Hide resolved
@pellared
Copy link
Member Author

CC @bogdandrutu

@pellared pellared mentioned this pull request Sep 11, 2024
Merged
@pellared pellared mentioned this pull request Sep 12, 2024
Closed
@pellared
Copy link
Member Author

@dashpole, @dmathieu, can you please take another look as there have been some API changes (related to #5791 (comment))?

@pellared pellared merged commit 23f7b41 into open-telemetry:main Sep 13, 2024
32 checks passed
@pellared pellared deleted the enabled-param branch September 13, 2024 04:35
@pellared pellared mentioned this pull request Sep 16, 2024
Merged
pellared added a commit to open-telemetry/opentelemetry-go-contrib that referenced this pull request Sep 17, 2024
@pellared
Bump otel/log after introducing EnabledParameters (#6115)
1bd570f 
Follows open-telemetry/opentelemetry-go#5791
which introduces a breaking change for the updated Go modules.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmathieu dmathieu dmathieu approved these changes

@dashpole dashpole dashpole approved these changes

@MrAlias MrAlias MrAlias approved these changes

@XSAM XSAM Awaiting requested review from XSAM XSAM is a code owner

@MadVikingGod MadVikingGod Awaiting requested review from MadVikingGod MadVikingGod is a code owner

@hanyuancheung hanyuancheung Awaiting requested review from hanyuancheung hanyuancheung is a code owner

Assignees

@pellared pellared

Labels
area:logs Part of OpenTelemetry logs pkg:API Related to an API package
Projects
Go: Logs (GA)
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Specs inconsistent Enabled API for Logger
4 participants
@pellared @dmathieu @dashpole @MrAlias