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

Map RUM PerformanceResourceTiming fields #9429

Merged
merged 2 commits into from
Oct 25, 2022
Merged

Conversation

axw
Copy link
Member

@axw axw commented Oct 25, 2022

Motivation/summary

Add field mappings for:

  • http.response.transfer_size
  • http.response.encoded_body_size
  • http.response.decoded_body_size

The fields are mapped with index: false, to minimise storage overhead. They can still be used in searches, just a bit slower.

The intake schema for these fields has been changed from float64 to int, as these are originally integer values.
See https://w3c.github.io/resource-timing/#sec-performanceresourcetiming

testdata, and hence approvals, have been updated to change the mock values from floats to integers.

Checklist

How to test these changes

  1. Send some RUM page-load transactions and HTTP spans to APM Server
  2. Ensure the fields can be searched and aggregated

Related issues

None

@axw axw added the v8.6.0 label Oct 25, 2022
@mergify
Copy link
Contributor

mergify bot commented Oct 25, 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 Oct 25, 2022
Add field mappings for:

 - http.response.transfer_size
 - http.response.encoded_body_size
 - http.response.decoded_body_size

The fields are mapped with `index: false`, to minimise storage overhead.
They can still be used in searches, just a bit slower.

The intake schema for these fields has been changed from float64 to int,
as these are originally integer values.
See https://w3c.github.io/resource-timing/#sec-performanceresourcetiming

testdata, and hence approvals, have been updated to change the mock
values from floats to integers.
@axw axw force-pushed the rum-size-fields branch from 66746a5 to 8622378 Compare October 25, 2022 02:00
@axw axw requested a review from a team October 25, 2022 02:01
@apmmachine
Copy link
Contributor

apmmachine commented Oct 25, 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-10-25T06:36:45.160+0000

  • Duration: 26 min 19 sec

Test stats 🧪

Test Results
Failed 0
Passed 154
Skipped 0
Total 154

🤖 GitHub comments

Expand to view the 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!)

@apmmachine
Copy link
Contributor

apmmachine commented Oct 25, 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/beater/request 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
BackendProcessor/spans.ndjson-12                                                                    90.8µs ±27%    113.8µs ±13%  +25.26%  (p=0.008 n=5+5)
RUMV3Processor/rum_events.ndjson-12                                                                 10.9µs ± 6%      9.6µs ±15%  -11.76%  (p=0.016 n=5+5)
BackendProcessorParallel/BenchmarkBackendProcessorParallel8/metricsets.ndjson-12                    4.99µs ± 1%     5.03µs ± 1%   +0.88%  (p=0.032 n=5+5)
BackendProcessorParallel/BenchmarkBackendProcessorParallel8/transactions-huge_traces.ndjson-12      5.61µs ± 1%     5.47µs ± 2%   -2.47%  (p=0.008 n=5+5)
BackendProcessorParallel/BenchmarkBackendProcessorParallel200/invalid-event-type.ndjson-12           807ns ± 3%      793ns ± 1%   -1.70%  (p=0.032 n=5+5)
BackendProcessorParallel/BenchmarkBackendProcessorParallel200/invalid-metadata-2.ndjson-12           493ns ± 1%      476ns ± 2%   -3.45%  (p=0.008 n=5+5)
BackendProcessorParallel/BenchmarkBackendProcessorParallel200/metricsets.ndjson-12                  4.43µs ± 1%     4.38µs ± 1%   -1.12%  (p=0.040 n=5+5)
ReadBatch/transactions_spans_rum.ndjson-12                                                          17.3µs ± 8%     20.5µs ±10%  +18.68%  (p=0.016 n=5+5)
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
AggregateTransaction-12                                                                             84.9ns ± 1%     84.0ns ± 1%   -1.12%  (p=0.032 n=5+5)
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

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/beater/request 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
BackendProcessorParallel/BenchmarkBackendProcessorParallel4/errors_rum.ndjson-12                    8.42kB ± 1%     8.56kB ± 2%   +1.73%  (p=0.016 n=5+5)
BackendProcessorParallel/BenchmarkBackendProcessorParallel4/logs.ndjson-12                          20.7kB ± 1%     20.9kB ± 1%   +1.07%  (p=0.008 n=5+5)
BackendProcessorParallel/BenchmarkBackendProcessorParallel4/metadata.ndjson-12                      5.40kB ± 2%     5.52kB ± 1%   +2.15%  (p=0.032 n=5+5)
BackendProcessorParallel/BenchmarkBackendProcessorParallel4/transactions-huge_traces.ndjson-12      15.9kB ± 1%     15.6kB ± 1%   -1.77%  (p=0.032 n=5+5)
BackendProcessorParallel/BenchmarkBackendProcessorParallel4/transactions_spans_rum_2.ndjson-12      6.16kB ± 1%     6.10kB ± 1%   -0.94%  (p=0.032 n=5+5)
ReadBatch/invalid-json-metadata.ndjson-12                                                           8.39kB ± 1%     8.49kB ± 1%   +1.10%  (p=0.008 n=5+5)
ReadBatch/otel-bridge.ndjson-12                                                                     10.0kB ± 0%     10.0kB ± 0%   +0.03%  (p=0.040 n=5+5)
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
ShardedWriteTransactionContended-12                                                                 8.03kB ±14%     6.91kB ± 5%  -13.92%  (p=0.008 n=5+5)
ReadEvents/json_codec/399_events-12                                                                 1.64MB ± 0%     1.64MB ± 0%   -0.15%  (p=0.016 n=5+4)

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/beater/request 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
BackendProcessorParallel/BenchmarkBackendProcessorParallel4/errors_transaction_id.ndjson-12            348 ± 0%        349 ± 0%   +0.29%  (p=0.029 n=4+4)
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

name                                                                                              old speed      new speed       delta
pkg:github.com/elastic/apm-server/internal/processor/stream goos:linux goarch:amd64
BackendProcessor/spans.ndjson-12                                                                  91.1MB/s ±32%   70.9MB/s ±12%  -22.20%  (p=0.008 n=5+5)
RUMV3Processor/rum_events.ndjson-12                                                                362MB/s ± 6%    412MB/s ±16%  +14.02%  (p=0.016 n=5+5)
BackendProcessorParallel/BenchmarkBackendProcessorParallel8/metricsets.ndjson-12                   510MB/s ± 1%    506MB/s ± 1%   -0.87%  (p=0.032 n=5+5)
BackendProcessorParallel/BenchmarkBackendProcessorParallel8/transactions-huge_traces.ndjson-12     565MB/s ± 1%    579MB/s ± 2%   +2.44%  (p=0.008 n=5+5)
BackendProcessorParallel/BenchmarkBackendProcessorParallel200/invalid-event-type.ndjson-12         485MB/s ± 3%    493MB/s ± 1%   +1.71%  (p=0.032 n=5+5)
BackendProcessorParallel/BenchmarkBackendProcessorParallel200/invalid-metadata-2.ndjson-12         885MB/s ± 1%    917MB/s ± 2%   +3.58%  (p=0.008 n=5+5)
BackendProcessorParallel/BenchmarkBackendProcessorParallel200/metricsets.ndjson-12                 574MB/s ± 1%    581MB/s ± 1%   +1.14%  (p=0.032 n=5+5)
ReadBatch/transactions_spans_rum.ndjson-12                                                        67.0MB/s ± 9%   56.7MB/s ±10%  -15.35%  (p=0.016 n=5+5)

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

@axw axw requested a review from a team October 25, 2022 02:41
@axw axw marked this pull request as ready for review October 25, 2022 02:42
Copy link
Member

@vigneshshanmugam vigneshshanmugam left a comment

Choose a reason for hiding this comment

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

LGTM from the RUM agent side.

@SylvainJuge
Copy link
Member

Regarding applying this change to other non-RUM agents, do we have (or plan to) do anything beyond updating the schema for now ? For example do we have to also attempt to capture similar attributes on HTTP spans ?

@axw axw deleted the rum-size-fields branch October 25, 2022 12:02
@axw
Copy link
Member Author

axw commented Oct 25, 2022

@SylvainJuge no, there's no plans for that at the moment

@carsonip carsonip self-assigned this Dec 5, 2022
@carsonip
Copy link
Member

carsonip commented Dec 5, 2022

Confirmed that we can search and aggregate on http.response.transfer_size, http.response.encoded_body_size and http.response.decoded_body_size and they are all integers. One thing I noticed is that decoded_body_size does not have unit: byte. Not sure if this is intended / affects any functionality.

In mapping:

          "response": {
            "properties": {
              "decoded_body_size": {
                "type": "long",
                "index": false
              },
              "encoded_body_size": {
                "type": "long",
                "index": false,
                "meta": {
                  "unit": "byte"
                }
              },
              "finished": {
                "type": "boolean"
              },
              "headers": {
                "type": "object"
              },
              "status_code": {
                "type": "long"
              },
              "transfer_size": {
                "type": "long",
                "index": false,
                "meta": {
                  "unit": "byte"
                }
              }
            }
          }

In code:
https://github.com/elastic/apm-server/pull/9429/files#diff-3d9ce867f164a3002e6829a4b35a8d9fafd3c3cd84f137c98fb1cedc97bcbe25R74

@axw
Copy link
Member Author

axw commented Dec 6, 2022

Good catch @carsonip. AFAIK it's not currently used, so it doesn't matter at the moment.

@elasticmachine
Copy link
Contributor

Package apm - 8.6.0-preview-1670294014 containing this change is available at https://epr.elastic.co/search?package=apm

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.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants