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

Removed duplication of request header holding the unique request id. #495

Closed
wants to merge 1 commit into from

Conversation

mcweba
Copy link
Collaborator

@mcweba mcweba commented Jan 25, 2023

The second header (x-rp-unique-id) was introduced back in 2016. However, I did not find a place where this duplicated header is actually used. To reduce confusion I removed this header.

@dominik-cnx
Copy link
Collaborator

I can find references to x-rp-unique-id in our project but do believe that we can remove those.

I wonder if we should change from

| x-request-id | unique id of the request when no x-rp-unique_id request header was provided

to

| x-request-id | unique id of the request, x-rp-unique_id if provided by the proxy or some gateleen generated id if the proxy doesn't provided the x-rp-unique_id.

The advantage would be that we always have some id in x-request-id

@hiddenalpha
Copy link
Member

hiddenalpha commented Feb 13, 2023

If I read code/comments around their occurrence it seems as this PR has chosen to go for the deprecated/old variant of the two. Reading the comments it sounds as the lodash variant is the older/deprecated one and the dash variant would be the newer one.

But that be said: I've no idea what those headers are for. Therefore:

  • I don't really care which one we remove ;)
  • Maybe a good idea to wait for an additional approval by someone who knows better.

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

Successfully merging this pull request may close these issues.

3 participants