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

Only add data_stream.* fields when enabled #4461

Merged
merged 2 commits into from
Nov 25, 2020
Merged

Conversation

axw
Copy link
Member

@axw axw commented Nov 25, 2020

Motivation/summary

Only add data_stream.* fields when apm-server.data_streams.enabled
is true. This avoids allocations when the feature is disabled, which will be
the default for some time yet.

Checklist

I have considered changes for:
- [ ] documentation
- [ ] logging (add log lines, choose appropriate log selector, etc.)
- [ ] metrics and monitoring (create issue for Kibana team to add metrics to visualizations, e.g. Kibana#44001)

How to test these changes

make test

Related issues

Closes #4460

@apmmachine
Copy link
Contributor

apmmachine commented Nov 25, 2020

💚 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

Expand to view the summary

Build stats

  • Build Cause: [Pull request #4461 updated]

  • Start Time: 2020-11-25T11:49:45.052+0000

  • Duration: 40 min 56 sec

Test stats 🧪

Test Results
Failed 0
Passed 4754
Skipped 143
Total 4897

Steps errors 3

Expand to view the steps failures

Compress

  • Took 0 min 0 sec . View more details on here
  • Description: tar --exclude=coverage-files.tgz -czf coverage-files.tgz coverage

Compress

  • Took 0 min 0 sec . View more details on here
  • Description: tar --exclude=system-tests-linux-files.tgz -czf system-tests-linux-files.tgz system-tests

Test Sync

  • Took 3 min 6 sec . View more details on here
  • Description: ./.ci/scripts/sync.sh

Only add data_streams.* fields when apm-server.data_streams.enabled
is true. This avoids allocations when the feature is disabled, which
will be the default for some time yet.
@axw axw marked this pull request as ready for review November 25, 2020 10:07
@axw axw requested a review from a team November 25, 2020 10:07
@codecov-io
Copy link

Codecov Report

Merging #4461 (f21d993) into master (576186d) will decrease coverage by 0.03%.
The diff coverage is 93.33%.

@@            Coverage Diff             @@
##           master    #4461      +/-   ##
==========================================
- Coverage   76.06%   76.03%   -0.04%     
==========================================
  Files         158      158              
  Lines        9782     9776       -6     
==========================================
- Hits         7441     7433       -8     
- Misses       2341     2343       +2     
Impacted Files Coverage Δ
publish/pub.go 73.80% <0.00%> (-2.10%) ⬇️
beater/beater.go 61.65% <100.00%> (-1.63%) ⬇️
model/error.go 98.60% <100.00%> (+<0.01%) ⬆️
model/metricset.go 94.91% <100.00%> (+0.08%) ⬆️
model/profile.go 100.00% <100.00%> (ø)
model/span.go 93.25% <100.00%> (+0.07%) ⬆️
model/transaction.go 85.18% <100.00%> (+0.27%) ⬆️
processor/otel/consumer.go 93.58% <0.00%> (-0.45%) ⬇️

@axw axw merged commit fe78d26 into elastic:master Nov 25, 2020
@axw axw deleted the data_streams_allocs branch November 25, 2020 12:31
jalvz pushed a commit to jalvz/apm-server that referenced this pull request Dec 10, 2020
Only add data_streams.* fields when apm-server.data_streams.enabled
is true. This avoids allocations when the feature is disabled, which
will be the default for some time yet.
axw added a commit that referenced this pull request Dec 14, 2020
Only add data_streams.* fields when apm-server.data_streams.enabled
is true. This avoids allocations when the feature is disabled, which
will be the default for some time yet.

Co-authored-by: Andrew Wilkins <axw@elastic.co>
@simitt
Copy link
Contributor

simitt commented Dec 23, 2020

This was an internal optimization, no user facing change - nothing to test manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Addition of data_stream fields causes allocations when data streams are disabled
4 participants