Good day,
While working on
https://github.com/open-telemetry/opentelemetry-go/pull/5858 I found
some other possible improvements.
This PR:
- Adds an early return to `SetAttributes` when no attributes are
provided.
- Only increases `s.attributes` to guarantee that there is enough space
for elements to be added.
- Fixes and issue where `truncateAttr` was not used when a attribute was
being updated in `addOverCapAttrs`.
Thanks for reviewing and please let me know if any changes are needed.
---------
Co-authored-by: Damien Mathieu <42@dmathieu.com>
The specification calls this filter "TraceBased":
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#tracebased
I don't have a huge preference between the two, but lean towards
TraceBasedFilter to match the specification, and to match our
environment variable configuration (and probably the future file
configuration).
This PR must be merged or closed prior to the v1.31 release, since we
can't change it after the release.
---------
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Part of https://github.com/open-telemetry/opentelemetry-go/issues/5249
This makes all existing types designed to implement the public Exemplar
API public by moving most of `internal/exemplar` to `exemplar`. The only
types that are not being made public are `exemplar.Drop`, and
`exemplar.FilteredReservoir`. Those types are moved to
`internal/aggregate`, and are renamed to `DropReservoir` and
`FilteredExemplarReservoir`.
The following types are made public:
* `exemplar.Exemplar`
* `exemplar.Filter`
* `exemplar.SampledFilter`
* `exemplar.AlwaysOnFilter`
* `exemplar.HistogramReservoir`
* `exemplar.FixedSizeReservoir`
* `exemplar.Reservoir`
* `exemplar.Value`
* `exemplar.ValueType`
Instead of using a global random number generator for all `randRes`,
have each value use its own. This removes the need for locking and
managing concurrent safe access to the global. Also, the field, given
the `Reservoir` type is not concurrent safe and the metric pipeline
guards this, does not need a `sync.Mutex` to guard it.
Supersedes https://github.com/open-telemetry/opentelemetry-go/pull/5815Fix#5814
### Performance Analysis
This change has approximately equivalent performance as the existing
code based on existing benchmarks.
```terminal
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/metric
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
│ old.txt │ new.txt │
│ sec/op │ sec/op vs base │
Exemplars/Int64Counter/8-8 14.00µ ± 3% 13.44µ ± 4% -3.98% (p=0.001 n=10)
│ old.txt │ new.txt │
│ B/op │ B/op vs base │
Exemplars/Int64Counter/8-8 3.791Ki ± 0% 3.791Ki ± 0% ~ (p=1.000 n=10) ¹
¹ all samples are equal
│ old.txt │ new.txt │
│ allocs/op │ allocs/op vs base │
Exemplars/Int64Counter/8-8 84.00 ± 0% 84.00 ± 0% ~ (p=1.000 n=10) ¹
¹ all samples are equal
```
### Added
- Support `OTEL_EXPORTER_OTLP_LOGS_INSECURE` and
`OTEL_EXPORTER_OTLP_INSECURE` environments in
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc`. (#5739)
- The `WithResource` option for `NewMeterProvider` now merges the
provided resources with the ones from environment variables. (#5773)
- The `WithResource` option for `NewLoggerProvider` now merges the
provided resources with the ones from environment variables. (#5773)
- Add UTF-8 support to `go.opentelemetry.io/otel/exporters/prometheus`.
(#5755)
### Fixed
- Fix memory leak in the global `MeterProvider` when identical
instruments are repeatedly created. (#5754)
- Fix panic instruments creation when setting meter provider. (#5758)
- Fix panic on instruments creation when setting meter provider. (#5758)
- Fix an issue where `SetMeterProvider` in `go.opentelemetry.io/otel`
might miss the delegation for instruments and registries. (#5780)
### Removed
- Drop support for [Go 1.21](https://go.dev/doc/go1.21). (#5736, #5740,
#5800)
With the `write` option, codespell fixes issues (which is a nice
behavior when we run it locally), but it also returns a 0 status code
(except if some failures couldn't be fixed).
So in order to actually fix the CI on a failing codespell, we need to
ensure the working directory is clean.
This release is the last to support [Go 1.21]. The next release will
require at least [Go 1.22].
### Added
- Add MacOS ARM64 platform to the compatibility testing suite. (#5577)
- Add `InstrumentationScope` field to `SpanStub` in
`go.opentelemetry.io/otel/sdk/trace/tracetest`, as a replacement for the
deprecated `InstrumentationLibrary`. (#5627)
- Make the initial release of
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc`. This new
module contains an OTLP exporter that transmits log telemetry using
gRPC. This module is unstable and breaking changes may be introduced.
See our [versioning policy](VERSIONING.md) for more information about
these stability guarantees. (#5629)
- Add `Walk` function to `TraceState` in
`go.opentelemetry.io/otel/trace` to iterate all the key-value pairs.
(#5651)
- Bridge the trace state in
`go.opentelemetry.io/otel/bridge/opencensus`. (#5651)
- Zero value of `SimpleProcessor` in `go.opentelemetry.io/otel/sdk/log`
no longer panics. (#5665)
- The `FilterProcessor` interface type is added in
`go.opentelemetry.io/otel/sdk/log/internal/x`. This is an optional and
experimental interface that log `Processor`s can implement to instruct
the `Logger` if a `Record` will be processed or not. It replaces the
existing `Enabled` method that is removed from the `Processor` interface
itself. It does not fall within the scope of the OpenTelemetry Go
versioning and stability [policy](./VERSIONING.md) and it may be changed
in backwards incompatible ways or removed in feature releases. (#5692)
- Support [Go 1.23]. (#5720)
### Changed
- `NewMemberRaw`, `NewKeyProperty` and `NewKeyValuePropertyRaw` in
`go.opentelemetry.io/otel/baggage` allow UTF-8 string in key. (#5132)
- `Processor.OnEmit` in `go.opentelemetry.io/otel/sdk/log` now accepts a
pointer to `Record` instead of a value so that the record modifications
done in a processor are propagated to subsequent registered processors.
(#5636)
- `SimpleProcessor.Enabled` in `go.opentelemetry.io/otel/sdk/log` now
returns `false` if the exporter is `nil`. (#5665)
- Update the concurrency requirements of `Exporter` in
`go.opentelemetry.io/otel/sdk/log`. (#5666)
- `SimpleProcessor` in `go.opentelemetry.io/otel/sdk/log` synchronizes
`OnEmit` calls. (#5666)
- The `Processor` interface in `go.opentelemetry.io/otel/sdk/log` no
longer includes the `Enabled` method. See the `FilterProcessor`
interface type added in `go.opentelemetry.io/otel/sdk/log/internal/x` to
continue providing this functionality. (#5692)
- The `SimpleProcessor` type in `go.opentelemetry.io/otel/sdk/log` is no
longer comparable. (#5693)
- The `BatchProcessor` type in `go.opentelemetry.io/otel/sdk/log` is no
longer comparable. (#5693)
### Fixed
- Correct comments for the priority of the `WithEndpoint` and
`WithEndpointURL` options and their corresponding environment variables
in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`.
(#5584)
- Pass the underlying error rather than a generic retry-able failure in
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`,
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` and
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`.
(#5541)
- Correct the `Tracer`, `Meter`, and `Logger` names used in
`go.opentelemetry.io/otel/example/dice`. (#5612)
- Correct the `Tracer` names used in
`go.opentelemetry.io/otel/example/namedtracer`. (#5612)
- Correct the `Tracer` name used in
`go.opentelemetry.io/otel/example/opencensus`. (#5612)
- Correct the `Tracer` and `Meter` names used in
`go.opentelemetry.io/otel/example/otel-collector`. (#5612)
- Correct the `Tracer` names used in
`go.opentelemetry.io/otel/example/passthrough`. (#5612)
- Correct the `Meter` name used in
`go.opentelemetry.io/otel/example/prometheus`. (#5612)
- Correct the `Tracer` names used in
`go.opentelemetry.io/otel/example/zipkin`. (#5612)
- Correct comments for the priority of the `WithEndpoint` and
`WithEndpointURL` options and their corresponding environment variables
in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc`
and `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`.
(#5641)
- Correct comments for the priority of the `WithEndpoint` and
`WithEndpointURL` options and their corresponding environment variables
in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`.
(#5650)
- Stop percent encoding header environment variables in
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`,
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`,
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` and
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`
(#5705)
- Remove invalid environment variable header keys in
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`,
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`,
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` and
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`
(#5705)
### Removed
- The `Enabled` method of the `SimpleProcessor` in
`go.opentelemetry.io/otel/sdk/log` is removed. (#5692)
- The `Enabled` method of the `BatchProcessor` in
`go.opentelemetry.io/otel/sdk/log` is removed. (#5692)
[Go 1.23]: https://go.dev/doc/go1.23
[Go 1.22]: https://go.dev/doc/go1.22
[Go 1.21]: https://go.dev/doc/go1.21
Closes#5425
Our current log `Processor` interface contains more functionality than
the [OTel
spec](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/sdk.md#logrecordprocessor-operations).
The additional functionality allows processors to report back to the API
if a Record should be constructed and emitted or not, which is quite
helpful[^1][^2][^3][^4][^5].
This removes the `Enabled` method from the `Processor` type. It adds
this functionality a new optional and experimental `FilterProcessor`
interface type. The logger and provider are updated to check for this
optional interface to be implemented with the configured processors and
uses them to back the `Logger.Enabled` method, preserving existing
functionality.
By making this change:
- The `Processor` interface is now compliant with the OTel spec and does
not contain any additional unspecified behavior.
- All `Processor` implementations are no longer required to implement an
`Enabled` method. The default, when they do not implement this method,
is to assume they are enabled.
### Benchmark
```terminal
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/log
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
│ old.txt │ new7.txt │
│ sec/op │ sec/op vs base │
LoggerEnabled-8 133.30n ± 3% 32.36n ± 3% -75.72% (p=0.000 n=10)
│ old.txt │ new7.txt │
│ B/op │ B/op vs base │
LoggerEnabled-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹
¹ all samples are equal
│ old.txt │ new7.txt │
│ allocs/op │ allocs/op vs base │
LoggerEnabled-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹
¹ all samples are equal
```
This is a significant performance improvement due to the `Record` no
longer being converted from the API version to the SDK version.
[^1]: https://pkg.go.dev/go.opentelemetry.io/contrib/processors/minsev
[^2]:
https://pkg.go.dev/go.opentelemetry.io/otel/sdk/log#BatchProcessor.Enabled
[^3]:
https://pkg.go.dev/go.opentelemetry.io/otel/sdk/log#SimpleProcessor.Enabled
[^4]:
af75717ac4/bridges/otelslog/handler.go (L206-L211)
[^5]:
d0309ddd8c/bridges/otelzap/core.go (L142-L146)
---------
Co-authored-by: Robert Pająk <pellared@hotmail.com>
Co-authored-by: Sam Xie <sam@samxie.me>
Go 1.23 was released a few days ago.
https://go.dev/blog/go1.23
This also upgrades golangci-lint (and fixes new issues), since it
appears 1.59 doesn't work nicely with Go 1.23.
---------
Co-authored-by: Sam Xie <sam@samxie.me>
Ensure the `Record` and `Provider` continue to be non-comparable.
Restrict `BatchProcessor` and `SimpleProcessor` to be non-comparable.
There is no obvious reason an end-user will need to compare these types,
and we want to keep the possibility of changing the internals without
changing this behavior.
if comparability is required by end-users in the future we can add
`Equal(other T)` methods in the future.
From
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/sdk.md#export:
> `Export` will never be called concurrently for the same exporter
instance.
From our SDK perspective this change will make
https://pkg.go.dev/go.opentelemetry.io/otel/exporters/stdout/stdoutlog
concurrent-safe when used with the simple processor. Before the change,
there can be multiple goroutines calling `Write` in parallel (via
`json.Encoder.Encode`).
### Remarks
[Maybe we should simply state that "whole" exporter implementation does
not need to be
concurrent-safe?](https://github.com/open-telemetry/opentelemetry-specification/pull/4173#discussion_r1699958752)
However:
1. we would need to make complex changes in `BatchExporter` to
synchronize the `Export`, `Shutdown`, `ForceFlush` calls
2. we would need to update all exporters (remove synchronization) and
simple exporter (add locks to `Shutdown`, `ForceFlush`)
3. I am 100% not sure if this would be compliant with the specification
- I think it would be complaint because we would simply give stronger
safety-measures
We should probably discuss it separately, but I wanted to highlight my
though process.
Even if we decide that simple and batch processors to synchronize all
calls then I would prefer to address it in a separate PR.
Related spec clarification PR:
-
https://github.com/open-telemetry/opentelemetry-specification/pull/4173
Rather than the deprecated InstrumentationLibrary
This is a replacement for
https://github.com/open-telemetry/opentelemetry-go/pull/3104, as after
this, there is no usage of `instrumentation.Library` within the SDK
anymore.
---------
Co-authored-by: Robert Pająk <pellared@hotmail.com>
Co-authored-by: Sam Xie <sam@samxie.me>
This benchmark was not only testing the batch processor, since it was
starting a span, and ending it.
So the entire process of recording span was being accounted.
This changes the benchmark to only account for the batch processor being
tested, not the entire stack.
Before:
```
pkg: go.opentelemetry.io/otel/sdk/trace
BenchmarkSpanProcessor-10 137320 8696 ns/op 11184 B/op 155 allocs/op
BenchmarkSpanProcessorVerboseLogging-10 135483 8881 ns/op 11184 B/op 155 allocs/op
PASS
ok go.opentelemetry.io/otel/sdk/trace 3.984s
```
After:
```
pkg: go.opentelemetry.io/otel/sdk/trace
BenchmarkSpanProcessorOnEnd/batch:_10,_spans:_10-10 6055572 173.2 ns/op 0 B/op 0 allocs/op
BenchmarkSpanProcessorOnEnd/batch:_10,_spans:_100-10 684236 1733 ns/op 0 B/op 0 allocs/op
BenchmarkSpanProcessorOnEnd/batch:_100,_spans:_10-10 6930391 173.8 ns/op 0 B/op 0 allocs/op
BenchmarkSpanProcessorOnEnd/batch:_100,_spans:_100-10 677128 1731 ns/op 0 B/op 0 allocs/op
BenchmarkSpanProcessorVerboseLogging-10 128823 9318 ns/op 11184 B/op 155 allocs/op
PASS
ok go.opentelemetry.io/otel/sdk/trace 6.763s
```
I haven't touched the verbose logging benchmark, as I suppose a
benchmark with verbose logging could actually benefit from the entire
stack.
I also feel one benchmark testing the entire stack up to there could be
nice.
Add a benchmark that shows that a processor that adds an attribute may
not introduce a heap allocation (as long as the log record still has <=
5 attributes).
### Added
- The `IsEmpty` method is added to the `Instrument` type in
`go.opentelemetry.io/otel/sdk/metric`.
This method is used to check if an `Instrument` instance is a
zero-value. (#5431)
- Store and provide the emitted `context.Context` in `ScopeRecords` of
`go.opentelemetry.io/otel/sdk/log/logtest`. (#5468)
- The `go.opentelemetry.io/otel/semconv/v1.26.0` package.
The package contains semantic conventions from the `v1.26.0` version of
the OpenTelemetry Semantic Conventions. (#5476)
- The `AssertRecordEqual` method to
`go.opentelemetry.io/otel/log/logtest` to allow comparison of two log
records in tests. (#5499)
- The `WithHeaders` option to
`go.opentelemetry.io/otel/exporters/zipkin` to allow configuring custom
http headers while exporting spans. (#5530)
### Changed
- `Tracer.Start` in `go.opentelemetry.io/otel/trace/noop` no longer
allocates a span for empty span context. (#5457)
- Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to
`go.opentelemetry.io/otel/semconv/v1.26.0` in
`go.opentelemetry.io/otel/example/otel-collector`. (#5490)
- Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to
`go.opentelemetry.io/otel/semconv/v1.26.0` in
`go.opentelemetry.io/otel/example/zipkin`. (#5490)
- Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to
`go.opentelemetry.io/otel/semconv/v1.26.0` in
`go.opentelemetry.io/otel/exporters/zipkin`. (#5490)
- The exporter no longer exports the deprecated "otel.library.name" or
"otel.library.version" attributes.
- Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to
`go.opentelemetry.io/otel/semconv/v1.26.0` in
`go.opentelemetry.io/otel/sdk/resource`. (#5490)
- Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to
`go.opentelemetry.io/otel/semconv/v1.26.0` in
`go.opentelemetry.io/otel/sdk/trace`. (#5490)
- `SimpleProcessor.OnEmit` in `go.opentelemetry.io/otel/sdk/log` no
longer allocates a slice which makes it possible to have a
zero-allocation log processing using `SimpleProcessor`. (#5493)
- Use non-generic functions in the `Start` method of
`"go.opentelemetry.io/otel/sdk/trace".Trace` to reduce memory
allocation. (#5497)
- `service.instance.id` is populated for a `Resource` created with
`"go.opentelemetry.io/otel/sdk/resource".Default` with a default value
when `OTEL_GO_X_RESOURCE` is set. (#5520)
- Improve performance of metric instruments in
`go.opentelemetry.io/otel/sdk/metric` by removing unnecessary calls to
`time.Now`. (#5545)
### Fixed
- Log a warning to the OpenTelemetry internal logger when a `Record` in
`go.opentelemetry.io/otel/sdk/log` drops an attribute due to a limit
being reached. (#5376)
- Identify the `Tracer` returned from the global `TracerProvider` in
`go.opentelemetry.io/otel/global` with its schema URL. (#5426)
- Identify the `Meter` returned from the global `MeterProvider` in
`go.opentelemetry.io/otel/global` with its schema URL. (#5426)
- Log a warning to the OpenTelemetry internal logger when a `Span` in
`go.opentelemetry.io/otel/sdk/trace` drops an attribute, event, or link
due to a limit being reached. (#5434)
- Document instrument name requirements in
`go.opentelemetry.io/otel/metric`. (#5435)
- Prevent random number generation data-race for experimental rand
exemplars in `go.opentelemetry.io/otel/sdk/metric`. (#5456)
- Fix counting number of dropped attributes of `Record` in
`go.opentelemetry.io/otel/sdk/log`. (#5464)
- Fix panic in baggage creation when a member contains `0x80` char in
key or value. (#5494)
- Correct comments for the priority of the `WithEndpoint` and
`WithEndpointURL` options and their corresponding environment variables
in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`.
(#5508)
- Retry trace and span ID generation if it generated an invalid one in
`go.opentelemetry.io/otel/sdk/trace`. (#5514)
- Fix stale timestamps reported by the last-value aggregation. (#5517)
- Indicate the `Exporter` in
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` must be
created by the `New` method. (#5521)
- Improved performance in all `{Bool,Int64,Float64,String}SliceValue`
functions of `go.opentelemetry.io/attributes` by reducing the number of
allocations. (#5549)
Towards https://github.com/open-telemetry/opentelemetry-go/issues/5065
Follow our own docs. From `Processor.Enabled` docs:
> Before modifying a Record, the implementation must use Record.Clone to
create a copy that shares no state with the original.
I was looking at the trace benchmarks, and noticed this one, which says
it tests "span start", but ending the span is included within the data.
So this change removes ending the span from the computation, and adds a
new benchmark which only computes span end.
benchstat for span start:
```
pkg: go.opentelemetry.io/otel/sdk/trace
│ bench-main │ bench-branch │
│ sec/op │ sec/op vs base │
TraceStart-10 725.6n ± 3% 667.2n ± 2% -8.04% (p=0.000 n=10)
│ bench-main │ bench-branch │
│ B/op │ B/op vs base │
TraceStart-10 704.0 ± 0% 704.0 ± 0% ~ (p=1.000 n=10) ¹
¹ all samples are equal
│ bench-main │ bench-branch │
│ allocs/op │ allocs/op vs base │
TraceStart-10 14.00 ± 0% 14.00 ± 0% ~ (p=1.000 n=10) ¹
¹ all samples are equal
```
Benchmark for span end:
```
BenchmarkSpanEnd-10 16486819 147.7 ns/op 0 B/op 0 allocs/op
```
