feat: Added support for logging of HTTP Headers #4803

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.

Sign up for GitHub

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

Jump to bottom

Merged

alanprot merged 26 commits into cortexproject:master from zspeaks:HTTPHeaderLogging

Sep 1, 2022

Contributor

zspeaks commented Jul 27, 2022 •

edited

Loading

What this PR does:
Adds support for the addition of the contents of HTTP headers to logs.
Which issue(s) this PR fixes:
Fixes #4799

Checklist

Tests updated
Documentation added
CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]


          feat: Added support for logging of HTTP Headers

7c93a11

Signed-off-by: Zach Speaks <zspeaks@amazon.com>

pull-request-size bot added the size/L label


          Fixed test failing due to namespace overlap

0926efb

Signed-off-by: Zach Speaks <zspeaks@amazon.com>

alvinlin123 reviewed

View reviewed changes

docs/configuration/config-file-reference.md Outdated Show resolved Hide resolved

alvinlin123 reviewed

View reviewed changes

pkg/api/middlewares_test.go Show resolved Hide resolved

alvinlin123 reviewed

View reviewed changes

pkg/api/middlewares_test.go Outdated Show resolved Hide resolved

alvinlin123 reviewed

View reviewed changes

pkg/util/log/log.go Outdated Show resolved Hide resolved

alvinlin123 reviewed

View reviewed changes

pkg/util/log/wrappers.go Outdated Show resolved Hide resolved

alvinlin123 reviewed

View reviewed changes

docs/configuration/config-file-reference.md Outdated Show resolved Hide resolved

Contributor

alvinlin123 commented Jul 27, 2022

@zspeaks can you also share an example how the actual log looks like?

alvinlin123 reviewed

View reviewed changes

pkg/util/log/wrappers.go Outdated Show resolved Hide resolved

Contributor Author

zspeaks commented Jul 28, 2022

Here's an example of what the logging looks like. Note: for the purposes of this example, I'm sending simulated requests with known headers and contents. In an actual running scenario, these additions to the logs would appear alongside the other information in the logs NOT on their own as they do here. With that said, here's examples for 1, 2 and 3 headers being included in the request.


          Updates to reflect PR feedback

8ada6b0

Signed-off-by: Zach Speaks <zspeaks@amazon.com>

zspeaks closed this

zspeaks force-pushed the HTTPHeaderLogging branch from 0926efb to a2f4d2c Compare

August 12, 2022 20:33

pull-request-size bot added size/XS and removed size/L labels

zspeaks added 2 commits

August 12, 2022 16:33


          Merge remote-tracking branch 'origin/HTTPHeaderLogging' into HTTPHead…

dfa9443

…erLogging

# Conflicts:
#	CHANGELOG.md


          Rough draft propogation to ingester + logging in ingester

Signed-off-by: Zach Speaks <zspeaks@amazon.com>

zspeaks reopened this

pull-request-size bot added size/L and removed size/XS labels

zspeaks added 2 commits

August 23, 2022 13:20


          Merge branch 'cortexproject:master' into HTTPHeaderLogging

d9b1907


          Merge branch 'cortexproject:master' into HTTPHeaderLogging

81a1d95

pull-request-size bot added size/XL and removed size/L labels

zspeaks and others added 4 commits

August 23, 2022 18:45


          Added Header Propogation to Ingester and Querier

d404f94

Added Header Propagation to Ingester and Querier
Signed-off-by: Zach Speaks <zspeaks@amazon.com>
Signed-off-by: Zachary Speaks <zspeaks@amazon.com>


          Addressed whitespace and linting issues

6429d04

Signed-off-by: Zach Speaks <zspeaks@amazon.com>
Signed-off-by: Zachary Speaks <zspeaks@amazon.com>


          Additional linting fixes

a61937c

Signed-off-by: Zach Speaks <zspeaks@amazon.com>
Signed-off-by: Zachary Speaks <zspeaks@amazon.com>


          Fixed failing DCO check

dc22472

Signed-off-by: Zachary Speaks <zspeaks@amazon.com>

danielblando reviewed

View reviewed changes

pkg/querier/queryrange/roundtrip.go Outdated Show resolved Hide resolved

danielblando reviewed

View reviewed changes

pkg/querier/worker/worker.go Outdated Show resolved Hide resolved

danielblando reviewed

View reviewed changes

pkg/util/grpcutil/util.go Outdated Show resolved Hide resolved

zspeaks added 3 commits

August 24, 2022 20:05


          Updates to reflect PR feedback

ddd90b9

Signed-off-by: Zachary Speaks <zspeaks@amazon.com>


          Additional Updates to reflect feedback

f183fda

Signed-off-by: Zachary Speaks <zspeaks@amazon.com>


          Improved string comparison when decoding

a10aefc

Signed-off-by: Zachary Speaks <zspeaks@amazon.com>

danielblando approved these changes

View reviewed changes

Contributor

danielblando left a comment

LGTM.
Just remember to update changelog

zspeaks commented

View reviewed changes

pkg/querier/worker/frontend_processor.go Outdated Show resolved Hide resolved


          Added header decoding to frontend_processor.go for when scheduler ser…

f617e45

…vice is not being used

Signed-off-by: Zachary Speaks <zspeaks@amazon.com>

alanprot reviewed

View reviewed changes

pkg/api/api.go Outdated Show resolved Hide resolved

alanprot reviewed

View reviewed changes

pkg/util/grpcclient/instrumentation.go Show resolved Hide resolved

zspeaks and others added 4 commits

August 26, 2022 16:12


          Merge branch 'cortexproject:master' into HTTPHeaderLogging

65effc7


          Added encoding/decoding interceptors for streams and updated changelog

8e5c3ba

Signed-off-by: Zachary Speaks <zspeaks@amazon.com>


          Undo accidental updates to single-process-config-block.yaml file

0e37794

Signed-off-by: Zachary Speaks <zspeaks@amazon.com>


          Whitespace updates

5800d4c

Signed-off-by: Zachary Speaks <zspeaks@amazon.com>

alanprot reviewed

View reviewed changes

pkg/api/middlewares.go Outdated Show resolved Hide resolved

alanprot reviewed

View reviewed changes

pkg/querier/queryrange/roundtrip.go Outdated Show resolved Hide resolved


          Refactored encode/decode headers to inject/extract to better align wi…

162ca7f

…th existing code

Signed-off-by: Zachary Speaks <zspeaks@amazon.com>

alvinlin123 approved these changes

View reviewed changes


          Fixed failing linter

d238040

Signed-off-by: Zachary Speaks <zspeaks@amazon.com>

alanprot reviewed

View reviewed changes

pkg/util/log/log.go Outdated

Comment on lines 155 to 160

+              func InjectHeadersIntoHTTPRequest(headerMap map[string]string, request *http.Request) {
+              	for header, contents := range headerMap {
+              		request.Header.Add(HeaderPropagationStringForRequestLogging, header)
+              		request.Header.Add(HeaderPropagationStringForRequestLogging, contents)
+              	}
+              }

Member

alanprot Aug 31, 2022

Could we pass the headers as is instead of creating a special header (httpheaderforwardingforlogging)?

zspeaks added 4 commits

August 31, 2022 16:19


          Switched extraction to being done by a middleware on the querier API

58bc638

Signed-off-by: Zachary Speaks <zspeaks@amazon.com>


          Linting

af7c250

Signed-off-by: Zachary Speaks <zspeaks@amazon.com>


          Changes to reflect PR feedback

Signed-off-by: Zachary Speaks <zspeaks@amazon.com>


          Updates to tests to match go conventions

1ebee10

Signed-off-by: Zachary Speaks <zspeaks@amazon.com>

alanprot approved these changes

View reviewed changes

Member

alanprot commented Sep 1, 2022

Nice!!

alanprot merged commit a795d53 into cortexproject:master

danielblando mentioned this pull request

Add log message for partial failure on push #4853

Merged

3 tasks

danielblando mentioned this pull request

Remove duplicate log message #4934

Merged

3 tasks

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

Labels