Get value from header or create with RequestIDHandler

[artefactop]

Hi,

I'm trying to understand how RequestIDHandler works. I configured the HTTP handler this way (got from https://github.com/rs/zerolog/blob/master/hlog/hlog.go#L150 and changed a bit):

func TestRequestIDFromHeaderHandler(t *testing.T) {
	out := &bytes.Buffer{}
	r := &http.Request{
		Header: http.Header{
			"Referer": []string{"http://foo.com/bar"},
			"X-Request-Id": []string{"514bbe5bb5251c92bd07a9846f4a1ab6"},
		},
	}
	h := RequestIDHandler("id", "X-Request-Id")(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		id, ok := IDFromRequest(r)
		if !ok {
			t.Fatal("Missing id in request")
		}
		if want, got := "514bbe5bb5251c92bd07a9846f4a1ab6", w.Header().Get("X-Request-Id"); got != want {
			t.Errorf("Invalid Request-Id header, got: %s, want: %s", got, want)
		}
		l := FromRequest(r)
		l.Log().Msg("")
		if want, got := fmt.Sprintf(`{"id":"%s"}`+"\n", id), decodeIfBinary(out); want != got {
			t.Errorf("Invalid log output, got: %s, want: %s", got, want)
		}
	}))
	h = NewHandler(zerolog.New(out))(h)
	h.ServeHTTP(httptest.NewRecorder(), r)
}

So if the request has already a header with that value, is ignored. But I see that the handler try to get the idKey from the context.

The problem I want to resolve if to create a new request ID only if it is not coming on the HTTP request.

Who is the responsible to set that value?

[gilcrest]

Hi @artefactop -

RequestIDHandler checks for the existence of the Request ID in the request context, if it's not there, it will create a new Request ID using @rs xid library and set that ID to the request context.

• RequestIDHandler adds the generated Request ID to the logger that is set to the Context with the value sent in the fieldKey parameter.
• RequestIDHandler will set the generated Request ID to the response headers based on the headerName parameter, it does not use this for anything else.

The inbound request headers have no bearing on the logic of this handler. If you want to check the inbound request headers for a value, you could add logic in your handler chain to only call this handler if you don't have a value in that header...

Sorry to jump in, hope I'm understanding what you're looking for and hope this explanation makes sense!

[artefactop]

Thank you, @gilcrest,

I do not understand why the following lines exists https://github.com/rs/zerolog/blob/master/hlog/hlog.go#L153

			ctx := r.Context()
			id, ok := IDFromRequest(r)
			if !ok {
				id = xid.New()
				ctx = context.WithValue(ctx, idKey{}, id)
				r = r.WithContext(ctx)
			}

As you explain before

RequestIDHandler adds the generated Request ID to the logger that is set to the Context with the value sent in the fieldKey parameter.

But it tries to get the value from the context, why if only that function sets that value?

My current handler chain is this one:

	handler := hlog.NewHandler(*logger)
	accessHandler := hlog.AccessHandler(func(r *http.Request, status, size int, duration time.Duration) {
		hlog.FromRequest(r).Info().
			Str("method", r.Method).
			Str("url", r.URL.String()).
			Int("status", status).
			Int("size", size).
			Dur("duration", duration).
			Msg("")
	})
	remoteAddrHandler := hlog.RemoteAddrHandler("ip")
	userAgentHandler := hlog.UserAgentHandler("user_agent")
	refererHandler := hlog.RefererHandler("referer")
	requestIDHandler := hlog.RequestIDHandler("req_id", "X-Request-Id")
	return handler(accessHandler(remoteAddrHandler(userAgentHandler(refererHandler(requestIDHandler(h))))))

I can create a custom handler that try to get the value from the header and then call hlog.RequestIDHandler if not exists inside it, but is a bit ugly because it will call a handler inside a handler.

What you think? Is there any possibility to change the requestIDHandler to be able to read the incoming header?

Like this:

func RequestIDHandler(fieldKey, headerName string) func(next http.Handler) http.Handler {
	return func(next http.Handler) http.Handler {
		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			ctx := r.Context()
			idStr := r.Header.Get(headerName)
			if idStr != "" {
				  id, ok := IDFromRequest(r)
				  if !ok {
					  idStr = xid.New().String()
					  ctx = context.WithValue(ctx, idKey{}, idStr)
					  r = r.WithContext(ctx)
				  }
			}
			if fieldKey != "" {
				log := zerolog.Ctx(ctx)
				log.UpdateContext(func(c zerolog.Context) zerolog.Context {
					return c.Str(fieldKey, idStr)
				})
			}
			if headerName != "" {
				w.Header().Set(headerName, idStr)
			}
			next.ServeHTTP(w, r)
		})
	}
}

[gilcrest]

@artefactop - I don't know if @rs would go for it, but I think the simplest solution would be to expose a function to allow adding your unique ID to the Context using the unexported context key idKey. I've added a function that would need to go in the hlog package below that I believe would work.

// IDToCtx adds a given string to the context
func IDToCtx(ctx context.Context, id string) context.Context {
	ctx = context.WithValue(ctx, idKey{}, id)
	return ctx
}

You could add the request ID you're pulling from the header to the request context using this function in a handler prior to calling RequestIDHandler, then it just falls in line, you don't have to change RequestIDHandler at all. This would also allow use of other keys than xid as well. Others may have a similar use case.

Thoughts?

Best,

Dan

[rs]

The name should be CtxWithID to follow the context.Context naming. I would accept such PR. Please make sure the handler would use this new method to insert the ID.

[gilcrest]

I'll put together a PR - thanks!