Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add support for configurable target header for the request_id middleware #2040

Merged
merged 1 commit into from
Dec 7, 2021
Merged

Conversation

luminoso
Copy link

@luminoso luminoso commented Dec 7, 2021

This PR adds the ability to choose what header to target for the X-Request-ID. Before this PR, the header used by the middleware was hardcoded for X-Request-ID, and exposing it via configuration will allow, among others, to set for X-Correlation-ID.

Setting a different header key is helpful for distributed traceability since X-Request-ID is used to trace single requests. In contrast, X-Correlation-ID(or others) is commonly used to trace multiple servers' transactions.

The default is set as the previous hardcoded header (X-Request-ID), which is a backward-compatible change.

Example for the new configuration:

(...)

rid = RequestIDWithConfig(RequestIDConfig{
		TargetHeader: echo.HeaderXCorrelationID,
})

(...)

This way they can be combined and the same middleware can be used for both use-cases.

Also updated documentation: labstack/echox#239

@codecov
Copy link

codecov bot commented Dec 7, 2021

Codecov Report

Merging #2040 (6dbf4d1) into master (b437ee3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2040   +/-   ##
=======================================
  Coverage   91.32%   91.33%           
=======================================
  Files          33       33           
  Lines        2871     2873    +2     
=======================================
+ Hits         2622     2624    +2     
  Misses        159      159           
  Partials       90       90           
Impacted Files Coverage Δ
echo.go 94.15% <ø> (ø)
middleware/request_id.go 85.71% <100.00%> (+1.50%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b437ee3...6dbf4d1. Read the comment docs.

@aldas
Copy link
Contributor

aldas commented Dec 7, 2021

LGTM.

sidenote: I would assume that if you are adding X-Correlation-ID you want to add X-Request-ID also. These headers are complementary to each other. Anyway this is ok, as you can always add other header with

	e.Use(RequestIDWithConfig(RequestIDConfig{
		RequestIDHandler: func(c echo.Context, s string) {
			corrID := c.Request().Header.Get(echo.HeaderXCorrelationID)
			if corrID != "" {
				c.Response().Header().Set(echo.HeaderXCorrelationID, corrID)
			}
		},
	}))

and there are probably cases when you have different header than X-Request-ID (whatever legacy or 3rdpary reasons) and would like to use middleware with your own custom header.

@aldas aldas merged commit c32fafa into labstack:master Dec 7, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants