Skip to content

log/logtest: change AssertEqual to accept TestingT for benchmark support#6908

Merged
pellared merged 11 commits intoopen-telemetry:mainfrom
flc1125:testing-tb
Jul 7, 2025
Merged

log/logtest: change AssertEqual to accept TestingT for benchmark support#6908
pellared merged 11 commits intoopen-telemetry:mainfrom
flc1125:testing-tb

Conversation

@flc1125
Copy link
Copy Markdown
Member

@flc1125 flc1125 commented Jun 17, 2025

This might just be a minor optimization, a non-functional change. Perhaps it can skip the CHANGELOG.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.8%. Comparing base (bd26298) to head (ffb47c2).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #6908   +/-   ##
=====================================
  Coverage   82.8%   82.8%           
=====================================
  Files        261     261           
  Lines      24312   24309    -3     
=====================================
- Hits       20142   20140    -2     
+ Misses      3795    3794    -1     
  Partials     375     375           
Files with missing lines Coverage Δ
log/logtest/assert.go 100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread log/logtest/assert_test.go Outdated
@dmathieu dmathieu added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jun 17, 2025
Copy link
Copy Markdown
Member

@dmathieu dmathieu left a comment

Choose a reason for hiding this comment

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

Skipping changelog, as this module isn't released yet.

@pellared
Copy link
Copy Markdown
Member

pellared commented Jun 17, 2025

I am not sure we are inn favor of this change.

Please see the conversation here: See: #6662 (comment)

My initial proposal was to expose TestingT interface to make it even easier to reuse as this would be a "thinner" interface (so that e.g. https://pkg.go.dev/github.com/goyek/goyek/v2#A would implement this interface).

// TestingT reports failure messages.
// *testing.T, *testing.B, *testing.F  implement this interface.
type TestingT interface {
	Errorf(format string, args ...any)
}

What are the benefits of using testing.TB over exposing our own interface?
How this change addresses concerns from the original proposal.

@flc1125
Copy link
Copy Markdown
Member Author

flc1125 commented Jun 17, 2025

TestingT

I also agree with exposing the TestingT interface. Apply the principle of least privilege. This way, control can be higher.

Comment thread log/logtest/assert.go Outdated
@pellared pellared changed the title refactor(log/logtest): update AssertEqual to accept testing.TB for benchmark support refactor(log/logtest): update AssertEqual to accept TestingT for benchmark support Jun 17, 2025
Copy link
Copy Markdown
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

I would love to have feedback from @MrAlias given our previous conversations: #6662 (comment)

Giving at least a week to respond.

@pellared

This comment was marked as resolved.

@pellared pellared removed the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jun 24, 2025
flc1125 added 3 commits June 25, 2025 13:34
- Change `AssertEqual` in `go.opentelemetry.io/otel/log/logtest` to accept `TestingT`
- This update allows the function to support both tests and benchmarks
- Change `AssertEqual` in `go.opentelemetry.io/otel/log/logtest` to accept `TestingT`
- This update allows the function to support both tests and benchmarks
@flc1125

This comment was marked as resolved.

@pellared pellared merged commit 186bd18 into open-telemetry:main Jul 7, 2025
32 checks passed
@pellared pellared changed the title refactor(log/logtest): update AssertEqual to accept TestingT for benchmark support log/logtest: change AssertEqual to accept TestingT for benchmark support Jul 7, 2025
@MrAlias MrAlias added this to the v1.38.0 milestone Jul 17, 2025
@MrAlias MrAlias mentioned this pull request Aug 29, 2025
MrAlias added a commit that referenced this pull request Aug 29, 2025
This release is the last to support [Go 1.23].
The next release will require at least [Go 1.24].

### Added

- Add native histogram exemplar support in
`go.opentelemetry.io/otel/exporters/prometheus`. (#6772)
- Add template attribute functions to the
`go.opentelmetry.io/otel/semconv/v1.34.0` package. (#6939)
  - `ContainerLabel`
  - `DBOperationParameter`
  - `DBSystemParameter`
  - `HTTPRequestHeader`
  - `HTTPResponseHeader`
  - `K8SCronJobAnnotation`
  - `K8SCronJobLabel`
  - `K8SDaemonSetAnnotation`
  - `K8SDaemonSetLabel`
  - `K8SDeploymentAnnotation`
  - `K8SDeploymentLabel`
  - `K8SJobAnnotation`
  - `K8SJobLabel`
  - `K8SNamespaceAnnotation`
  - `K8SNamespaceLabel`
  - `K8SNodeAnnotation`
  - `K8SNodeLabel`
  - `K8SPodAnnotation`
  - `K8SPodLabel`
  - `K8SReplicaSetAnnotation`
  - `K8SReplicaSetLabel`
  - `K8SStatefulSetAnnotation`
  - `K8SStatefulSetLabel`
  - `ProcessEnvironmentVariable`
  - `RPCConnectRPCRequestMetadata`
  - `RPCConnectRPCResponseMetadata`
  - `RPCGRPCRequestMetadata`
  - `RPCGRPCResponseMetadata`
- Add `ErrorType` attribute helper function to the
`go.opentelmetry.io/otel/semconv/v1.34.0` package. (#6962)
- Add `WithAllowKeyDuplication` in `go.opentelemetry.io/otel/sdk/log`
which can be used to disable deduplication for log records. (#6968)
- Add `WithCardinalityLimit` option to configure the cardinality limit
in `go.opentelemetry.io/otel/sdk/metric`. (#6996, #7065, #7081, #7164,
#7165, #7179)
- Add `Clone` method to `Record` in `go.opentelemetry.io/otel/log` that
returns a copy of the record with no shared state. (#7001)
- Add experimental self-observability span and batch span processor
metrics in `go.opentelemetry.io/otel/sdk/trace`. Check the
`go.opentelemetry.io/otel/sdk/trace/internal/x` package documentation
for more information. (#7027, #6393, #7209)
- The `go.opentelemetry.io/otel/semconv/v1.36.0` package. The package
contains semantic conventions from the `v1.36.0` version of the
OpenTelemetry Semantic Conventions. See the [migration
documentation](./semconv/v1.36.0/MIGRATION.md) for information on how to
upgrade from `go.opentelemetry.io/otel/semconv/v1.34.0.`(#7032, #7041)
- Add support for configuring Prometheus name translation using
`WithTranslationStrategy` option in
`go.opentelemetry.io/otel/exporters/prometheus`. The current default
translation strategy when UTF-8 mode is enabled is
`NoUTF8EscapingWithSuffixes`, but a future release will change the
default strategy to `UnderscoreEscapingWithSuffixes` for compliance with
the specification. (#7111)
- Add experimental self-observability log metrics in
`go.opentelemetry.io/otel/sdk/log`. Check the
`go.opentelemetry.io/otel/sdk/log/internal/x` package documentation for
more information. (#7121)
- Add experimental self-observability trace exporter metrics in
`go.opentelemetry.io/otel/exporters/stdout/stdouttrace`. Check the
`go.opentelemetry.io/otel/exporters/stdout/stdouttrace/internal/x`
package documentation for more information. (#7133)
- Support testing of [Go 1.25]. (#7187)
- The `go.opentelemetry.io/otel/semconv/v1.37.0` package. The package
contains semantic conventions from the `v1.37.0` version of the
OpenTelemetry Semantic Conventions. See the [migration
documentation](./semconv/v1.37.0/MIGRATION.md) for information on how to
upgrade from `go.opentelemetry.io/otel/semconv/v1.36.0.`(#7254)

### Changed

- Optimize `TraceIDFromHex` and `SpanIDFromHex` in
`go.opentelemetry.io/otel/sdk/trace`. (#6791)
- Change `AssertEqual` in `go.opentelemetry.io/otel/log/logtest` to
accept `TestingT` in order to support benchmarks and fuzz tests. (#6908)
- Change `DefaultExemplarReservoirProviderSelector` in
`go.opentelemetry.io/otel/sdk/metric` to use `runtime.GOMAXPROCS(0)`
instead of `runtime.NumCPU()` for the `FixedSizeReservoirProvider`
default size. (#7094)

### Fixed

- `SetBody` method of `Record` in `go.opentelemetry.io/otel/sdk/log` now
deduplicates key-value collections (`log.Value` of `log.KindMap` from
`go.opentelemetry.io/otel/log`). (#7002)
- Fix `go.opentelemetry.io/otel/exporters/prometheus` to not append a
suffix if it's already present in metric name. (#7088)
- Fix the `go.opentelemetry.io/otel/exporters/stdout/stdouttrace`
self-observability component type and name. (#7195)
- Fix partial export count metric in
`go.opentelemetry.io/otel/exporters/stdout/stdouttrace`. (#7199)

### Deprecated

- Deprecate `WithoutUnits` and `WithoutCounterSuffixes` options,
preferring `WithTranslationStrategy` instead. (#7111)
- Deprecate support for `OTEL_GO_X_CARDINALITY_LIMIT` environment
variable in `go.opentelemetry.io/otel/sdk/metric`. Use
`WithCardinalityLimit` option instead. (#7166)
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.

4 participants