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

Jaeger exporter still creates too big UDP packets #2503

Closed
Jownathan opened this issue Jan 10, 2022 · 1 comment · Fixed by #2512
Closed

Jaeger exporter still creates too big UDP packets #2503

Jownathan opened this issue Jan 10, 2022 · 1 comment · Fixed by #2512
Labels
bug Something isn't working
Milestone

Comments

@Jownathan
Copy link

Description

This bug report is similar to a previous bug report, this bug was fixed in this PR but the issue remains, potentially because of a small mistake in the PR.

The original bug was:

Jaeger exporter logs errors like these: 'data does not fit within one UDP packet; size 65006, max 65000, spans 440'. This leads to spans missing from our traces.

But after the PR the bug still occurs and the logs now look as follows:

'data does not fit within one UDP packet; size 64941, max 64930, spans 435'

As you can see, the max went from 65000 to 64930. This is because of the new emitBatchOverhead of 70 introduced in the PR.
It seems that the emitBatchOverhead variable is not only used for limiting the amount of spans that get added (which is what was required to fix the original issue) but it is also used for the maximum size of an UDP packet (which is incorrect).
As seen in the error message max 64930 should be max 65000, which would fix the issue.

Environment

  • OS: alpine base image
  • Architecture: x86
  • Go Version: 1.17
  • opentelemetry-go versions:
    go.opentelemetry.io/otel v1.3.1-0.20220104180153-776bfdcb2358
    go.opentelemetry.io/otel/exporters/jaeger v1.3.1-0.20220104180153-776bfdcb2358
    go.opentelemetry.io/otel/trace v1.3.1-0.20220104180153-776bfdcb2358

Steps To Reproduce

  1. Produce traces with a thousand spans.
  2. Error 'data does not fit within one UDP packet; size 64941, max 64930, spans 435' is logged occasionally.

Expected behavior

Jaeger exporter does not create UDP packets that are too big.

@Jownathan Jownathan added the bug Something isn't working label Jan 10, 2022
MrAlias pushed a commit that referenced this issue Jan 18, 2022
#2512)

* emitBatchOverhead should only be used for splitting spans into batches (#2503)

* limit max packet size parameter
@jflipts
Copy link

jflipts commented Jan 20, 2022

This fix seems to have resolved the issue, thank you!

MrAlias added a commit that referenced this issue Mar 21, 2022
* Allow setting the Sampler via environment variables (#2305)

* Add changelog entry.

* Replace t.Setenv with internaltest/SetEnvVariables for Go <= 1.6.

* Handle the lack of a sampler argument without logging errors.

* Add additional test cases and error checks.

* Refactor documentation.

Co-authored-by: Joshua MacDonald <[email protected]>

* emitBatchOverhead should only be used for splitting spans into batches (#2512)

* emitBatchOverhead should only be used for splitting spans into batches (#2503)

* limit max packet size parameter

* Add additional errors types, simplify abstractions and error handling

* Make error comparisons less fragile.

* Fix typo in jaeger example (#2524)

* Fix some typos in docs for Go libraries (#2520)

Co-authored-by: Tyler Yahn <[email protected]>

* Fix getting-started.md Run function (#2527)

* Fix getting-started.md Run function, it assigns this new context to a variable shared between connections in to accept loop. Thus creating a growing chain of contexts. so every calculate fibonacci request, all spans in a trace.

* add a comment explaining the reason for that new variable

* update example fib

* Bump github.com/google/go-cmp from 0.5.6 to 0.5.7 across the project (#2545)

* update go-cmp to 0.5.7

* fixes go.sums

Co-authored-by: Aaron Clawson <[email protected]>

* Un-escape url coding when parsing baggage. (#2529)

* un-escape url coding when parsing baggage.

* Added changelog

Co-authored-by: Aaron Clawson <[email protected]>
Co-authored-by: Tyler Yahn <[email protected]>

* Bump go.opentelemetry.io/proto/otlp from 0.11.0 to 0.12.0 (#2546)

* Update go.opentelemetry.io/proto/otlp to v0.12.0

* Changelog

* Update CHANGELOG.md

Fix's md linting

Co-authored-by: Tyler Yahn <[email protected]>

Co-authored-by: Aaron Clawson <[email protected]>
Co-authored-by: Tyler Yahn <[email protected]>

* Remove unused sdk/internal/santize (#2549)

* Add links to code examples and docs (#2551)

* Bump github.com/prometheus/client_golang from 1.11.0 to 1.12.0 in /exporters/prometheus (#2541)

* Bump github.com/prometheus/client_golang in /exporters/prometheus

Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.11.0 to 1.12.0.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.11.0...v1.12.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* go mod tidy

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Tyler Yahn <[email protected]>
Co-authored-by: Tyler Yahn <[email protected]>

* Optimize evictedQueue implementation and use (#2556)

* Optimize evictedQueue impl and use

Avoid unnecessary allocations in the recordingSpan by using an
evictedQueue type instead of a pointer to one.

Lazy allocate the evictedQueue queue to prevent unnecessary operations
for spans without any use of the queue.

Document the evictedQueue

* Fix grammar

* Add env support for batch span processor (#2515)

* Add env support for batch span processor

* Update changelog

* lint

* Bump golang.org/x/tools from 0.1.8 to 0.1.9 in /internal/tools (#2566)

* Bump golang.org/x/tools from 0.1.8 to 0.1.9 in /internal/tools

Bumps [golang.org/x/tools](https://github.com/golang/tools) from 0.1.8 to 0.1.9.
- [Release notes](https://github.com/golang/tools/releases)
- [Commits](golang/tools@v0.1.8...v0.1.9)

---
updated-dependencies:
- dependency-name: golang.org/x/tools
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* Auto-fix go.sum changes in dependent modules

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: MrAlias <[email protected]>

* Bump github.com/golangci/golangci-lint from 1.43.0 to 1.44.0 in /internal/tools (#2567)

* Bump github.com/golangci/golangci-lint in /internal/tools

Bumps [github.com/golangci/golangci-lint](https://github.com/golangci/golangci-lint) from 1.43.0 to 1.44.0.
- [Release notes](https://github.com/golangci/golangci-lint/releases)
- [Changelog](https://github.com/golangci/golangci-lint/blob/master/CHANGELOG.md)
- [Commits](golangci/golangci-lint@v1.43.0...v1.44.0)

---
updated-dependencies:
- dependency-name: github.com/golangci/golangci-lint
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Auto-fix go.sum changes in dependent modules

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: MrAlias <[email protected]>

* Bump github.com/prometheus/client_golang from 1.12.0 to 1.12.1 in /exporters/prometheus (#2570)

* Bump github.com/prometheus/client_golang in /exporters/prometheus

Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.12.0 to 1.12.1.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.12.0...v1.12.1)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* Auto-fix go.sum changes in dependent modules

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: MrAlias <[email protected]>

* Fix TestBackoffRetry in otlp/internal/retry package (#2562)

* Fix TestBackoffRetry in otlp retry pkg

The delay of the retry is within two times a randomization factor (the
back-off time is delay * random number within [1 - factor, 1 + factor].
This means the waitFunc in TestBackoffRetry needs to check the delay is
within an appropriate delta, not equal to configure initial delay.

* Fix delta value

* Fix delta

Co-authored-by: Aaron Clawson <[email protected]>

* Bump google.golang.org/grpc from 1.43.0 to 1.44.0 in /exporters/otlp/otlptrace (#2568)

* Bump google.golang.org/grpc in /exporters/otlp/otlptrace

Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.43.0 to 1.44.0.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.43.0...v1.44.0)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Auto-fix go.sum changes in dependent modules

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: MrAlias <[email protected]>

* Bump google.golang.org/grpc from 1.43.0 to 1.44.0 in /example/otel-collector (#2565)

* Bump google.golang.org/grpc in /example/otel-collector

Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.43.0 to 1.44.0.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.43.0...v1.44.0)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Auto-fix go.sum changes in dependent modules

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: MrAlias <[email protected]>

* Bump google.golang.org/grpc from 1.43.0 to 1.44.0 in /exporters/otlp/otlpmetric (#2572)

* Bump google.golang.org/grpc in /exporters/otlp/otlpmetric

Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.43.0 to 1.44.0.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.43.0...v1.44.0)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Auto-fix go.sum changes in dependent modules

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: MrAlias <[email protected]>

* Change Options to accept type not pointer (#2558)

* Change trace options to accept type not pointer

Add benchmark to show allocation improvement.

* Update CONTRIBUTING.md guidelines

* Update all Option iface

* Fix grammar in CONTRIBUTING

* Do not store TracerProvider or Tracer fields in SDK recordingSpan (#2575)

* Do not store TracerProvider fields in span

Instead of keeping a reference to the span's Tracer, and therefore also
it's TracerProvider, and the associated resource and spanLimits just
keep the reference to the Tracer. Refer to the TracerProvider fields
when needed instead.

* Make span refer to the inst lib via the Tracer

Instead of holding a field in the span, refer to the field in the parent
Tracer.

* [website_docs] fix page meta-links (#2580)

Contributes to open-telemetry/opentelemetry.io#1096

/cc @cartermp @austinlparker

* Validate members once, in `NewMember` (#2522)

* use NewMember, or specify if the member is not validated when creating new ones

* expect members to already be validated when creating a new package

* add changelog entry

* add an isEmpty field to member and property for quick validation

* rename isEmpty to hasData

So by default, an empty struct really is marked as having no data

* Update baggage/baggage_test.go

Co-authored-by: Aaron Clawson <[email protected]>

* don't validate the member in parseMember, we alredy ran that validation

We also don't want to use NewMember, as that runs the property
validation again, making the benchmark quite slower

* move changelog entry to the fixed section

* provide the member/property data when returning an invalid error

Co-authored-by: Aaron Clawson <[email protected]>

* Fix link to Zipkin exporter (#2581)

Currently it is linked to the old package that was moved.

* Unexport EnvBatchSpanProcessor* constants (#2583)

* Move BSP env support to internal

* Use pkg name

* Update env test

* Use internal/env in sdk/trace

* Avoid an extra allocation in applyTracerProviderEnvConfigs.

* Add additional errors for ratio > 1.0.

* Add test cases for ratio > 1.0.

* Update CHANGELOG.md

Co-authored-by: Joshua MacDonald <[email protected]>
Co-authored-by: jaychung <[email protected]>
Co-authored-by: Ben Wells <[email protected]>
Co-authored-by: Jeremy Kaplan <[email protected]>
Co-authored-by: Tyler Yahn <[email protected]>
Co-authored-by: thinkgo <[email protected]>
Co-authored-by: Aaron Clawson <[email protected]>
Co-authored-by: Aaron Clawson <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Tyler Yahn <[email protected]>
Co-authored-by: Chao Weng <[email protected]>
Co-authored-by: Patrice Chalin <[email protected]>
Co-authored-by: Damien Mathieu <[email protected]>
@pellared pellared added this to the untracked milestone Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants