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

Update opentelemetry-collector modules to v0.56.0 #8765

Merged
merged 5 commits into from
Aug 2, 2022

Conversation

axw
Copy link
Member

@axw axw commented Aug 1, 2022

Motivation/summary

The first commit bumps the OTel module dependencies, and vendored internal/otel_collector code. The second adjusts apm-server code, the bulk of which is just adjusting to API changes, but there are some functional changes noted below. Finally, system tests are updated to the latest OTel SDK.

open-telemetry/opentelemetry-collector-contrib#10273 uncovered an issue where we were recording the "event" tag as a label, when we should have been recording it as the "message" field in some circumstances.

open-telemetry/opentelemetry-proto#373 removed the deprecated LogRecord.Name field. There is no replacement, hence we no longer record event.action.

Checklist

How to test these changes

Check that OpenTelemetry Swift 1.3 works with the server.

Related issues

Closes #8576

@axw axw added the v8.5.0 label Aug 1, 2022
@mergify
Copy link
Contributor

mergify bot commented Aug 1, 2022

This pull request does not have a backport label. Could you fix it @axw? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-7.x is the label to automatically backport to the 7.x branch.
  • backport-7./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Aug 1, 2022
@axw axw force-pushed the update-otel-collector branch from 612fb2d to f0ec322 Compare August 1, 2022 08:58
@apmmachine
Copy link
Contributor

apmmachine commented Aug 1, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-08-02T07:49:35.383+0000

  • Duration: 25 min 0 sec

Test stats 🧪

Test Results
Failed 0
Passed 137
Skipped 1
Total 138

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate and publish the docker images.

  • /test windows : Build & tests on Windows.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@axw axw force-pushed the update-otel-collector branch from f0ec322 to c2a0163 Compare August 1, 2022 09:12
@apmmachine
Copy link
Contributor

apmmachine commented Aug 1, 2022

📚 Go benchmark report

Diff with the main branch

name                                                            old time/op    new time/op    delta
pkg:github.com/elastic/apm-server/internal/agentcfg goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/decoder goos:linux goarch:amd64
CompressedRequestReader/gzip_sniff-12                             17.6µs ± 2%    13.7µs ±67%  -21.76%  (p=0.032 n=5+5)
pkg:github.com/elastic/apm-server/internal/model/modelindexer goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/processor/stream goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/publish goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/spanmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/txmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling/eventstorage goos:linux goarch:amd64
ReadEvents/json_codec/1000_events-12                              6.48ms ± 2%    6.80ms ± 6%   +4.89%  (p=0.032 n=5+5)
ReadEvents/nop_codec/1000_events-12                                849µs ± 8%     953µs ± 8%  +12.22%  (p=0.032 n=5+5)

name                                                            old alloc/op   new alloc/op   delta
pkg:github.com/elastic/apm-server/internal/agentcfg goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/decoder goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/model/modelindexer goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/processor/stream goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/publish goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/spanmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/txmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling/eventstorage goos:linux goarch:amd64
ReadEvents/json_codec/100_events-12                                751kB ± 0%     750kB ± 0%   -0.06%  (p=0.032 n=5+4)
ReadEvents/nop_codec/399_events-12                                 899kB ± 0%     897kB ± 0%   -0.26%  (p=0.032 n=5+5)

name                                                            old allocs/op  new allocs/op  delta
pkg:github.com/elastic/apm-server/internal/agentcfg goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/decoder goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/model/modelindexer goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/processor/stream goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/publish goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/spanmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/txmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling/eventstorage goos:linux goarch:amd64

report generated with https://pkg.go.dev/golang.org/x/perf/cmd/benchstat

@axw axw force-pushed the update-otel-collector branch 3 times, most recently from 3966771 to ae6c52b Compare August 1, 2022 12:38
axw added 3 commits August 1, 2022 20:49
Mostly just adjusting to API changes, but there are
some functional changes related to Jaeger span events.
open-telemetry/opentelemetry-collector-contrib#10273
uncovered an issue where we were recording the "event" tag
as a label, when we should have been recording it as the
"message" field in some circumstances.
@axw axw force-pushed the update-otel-collector branch from ae6c52b to bff66c5 Compare August 1, 2022 12:49
@axw axw marked this pull request as ready for review August 2, 2022 00:20
@axw axw requested a review from a team August 2, 2022 00:20
@mergify
Copy link
Contributor

mergify bot commented Aug 2, 2022

This pull request is now in conflicts. Could you fix it @axw? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b update-otel-collector upstream/update-otel-collector
git merge upstream/main
git push upstream update-otel-collector

Copy link
Contributor

@simitt simitt left a comment

Choose a reason for hiding this comment

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

LGTM!

I did not conduct any manual testing, but suggest that we ask mobile developers to test against the changes after merging.

@axw axw enabled auto-merge (squash) August 2, 2022 07:49
@axw axw merged commit addfc87 into elastic:main Aug 2, 2022
@axw axw added the test-plan label Aug 2, 2022
@axw axw deleted the update-otel-collector branch August 2, 2022 10:11
@lahsivjar lahsivjar self-assigned this Sep 27, 2022
@lahsivjar
Copy link
Contributor

Tested with 8.5.0-BC1 by using opentelemetry-swift examples. APM-Server is able to ingest data:

Screenshot 2022-09-28 at 1 27 43 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip Skip notification from the automated backport with mergify test-plan test-plan-ok v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update OTel Collector dependency >= 0.56.0
4 participants