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

Introduce apm-server.response_headers config #4523

Merged
merged 4 commits into from
Dec 14, 2020
Merged

Conversation

axw
Copy link
Member

@axw axw commented Dec 14, 2020

Motivation/summary

Like apm-server.rum.response_headers, but for all HTTP responses. If both are defined, then they will both be added to RUM responses, with values for the same key concatenated.

Checklist

I have considered changes for:

How to test these changes

cd systemtest && go test -v

Related issues

Closes #4522

axw added 3 commits December 14, 2020 09:38
Like apm-server.rum.response_headers, but for all
HTTP responses. If both are defined, then they will
both be added to RUM responses.
@axw axw marked this pull request as ready for review December 14, 2020 01:58
@apmmachine
Copy link
Contributor

apmmachine commented Dec 14, 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 #4523 updated

  • Start Time: 2020-12-14T07:42:11.150+0000

  • Duration: 53 min 25 sec

Test stats 🧪

Test Results
Failed 0
Passed 4624
Skipped 139
Total 4763

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 50 sec . View more details on here
  • Description: ./.ci/scripts/sync.sh

@codecov-io
Copy link

Codecov Report

Merging #4523 (ad86b11) into master (c18c658) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4523      +/-   ##
==========================================
- Coverage   75.94%   75.92%   -0.03%     
==========================================
  Files         161      161              
  Lines        9783     9786       +3     
==========================================
  Hits         7430     7430              
- Misses       2353     2356       +3     
Impacted Files Coverage Δ
beater/config/config.go 68.33% <ø> (ø)
beater/api/mux.go 90.90% <100.00%> (+0.32%) ⬆️
kibana/connecting_client.go 64.91% <0.00%> (-5.27%) ⬇️

@axw axw requested a review from a team December 14, 2020 02:58
@axw axw merged commit 750ea89 into elastic:master Dec 14, 2020
@axw axw deleted the response-headers branch December 14, 2020 08:37
axw added a commit to axw/apm-server that referenced this pull request Dec 15, 2020
* Introduce apm-server.response_headers config

Like apm-server.rum.response_headers, but for all
HTTP responses. If both are defined, then they will
both be added to RUM responses.
simitt pushed a commit that referenced this pull request Dec 15, 2020
Like apm-server.rum.response_headers, but for all
HTTP responses. If both are defined, then they will
both be added to RUM responses.
@simitt
Copy link
Contributor

simitt commented Dec 23, 2020

Tested with BC1 - works as expected.

@axw response headers are not applied to the Jaeger http endpoint but I assume that's also not required.

@simitt simitt self-assigned this Dec 23, 2020
@axw
Copy link
Member Author

axw commented Dec 24, 2020

@axw response headers are not applied to the Jaeger http endpoint but I assume that's also not required.

Hmm I had forgotten about that TBH. I think that's OK for the moment since the Jaeger has its own config, and we'll add it on if/when we mux them.

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.

Configurable response headers for all responses
5 participants