3scale / APIcast

3scale API Gateway
Apache License 2.0
304 stars 171 forks source link

Introduce transaction_id policy #1460

Closed tkan145 closed 1 month ago

tkan145 commented 2 months ago

What

This PR partially support https://issues.redhat.com/browse/THREESCALE-10973. Specially it satisfy the following 2 requirements from the FAPI - baseline profile

  • shall set the response header x-fapi-interaction-id to the value received from the corresponding FAPI client request header or to a RFC4122 UUID value if the request header was not provided to track the interaction, e.g., x-fapi-interaction-id: c770aef3-6784-41f7-8e0e-ff5f97bddb3a;
  • shall log the value of x-fapi-interaction-id in the log entry; and

Why making a new policy and not extend the current Header Modification policy

We can easily extend the current Header Policy and add uuid filter and new option to generate the header in response if it does not exist in request. However, in my opinion this would complicate the logic of the Title Modification policy. For example, If the user sets the include_in_response checkbox while also setting a header in the Response field, and upstream also returns the same header, which header takes precedence?

Also the benefit of having separate policy so we can later extend the support algorithm if need (currently only uuidv4 is supported)

Answer to questions from https://issues.redhat.com/browse/THREESCALE-6577

what if an "XXXXX" (to be defined ID header) is already assigned to the incoming request (e.g. by ingress, or LB or somewhere previously) should that be used instead?

The policy won't modify the header if it exist

Should that be logged by apicast Is there any interaction with logging policy?

Yes, user should be able to log the header with Logging policy

What if there are multiple apicasts behind an LB, then they can't just use a counter to ensure uniqueness and an apicast GUID would be required.

We use UUID to the change of collision is fairly small

Do this, even if OpenTracing is being used?

I don't have answer to this but I think it's still useful to have this policy as not everyone use OpenTracing.

Verification steps:

"GET /test?user_key= HTTP/1.1" to service 1 and  with ID 8806c18d-c71d-4306-8eb3-43a8b538b31a - response: 8806c18d-c71d-4306-8eb3-43a8b538b31a

The response header should also contain the X-Transaction-ID header

HTTP/1.1 200 OK
Server: openresty
Date: Wed, 01 May 2024 08:47:39 GMT
Content-Type: application/json
Content-Length: 642
Connection: keep-alive
x-3scale-echo-api: echo-api/1.0.3
vary: Origin
x-content-type-options: nosniff
x-envoy-upstream-service-time: 0
X-Transaction-ID: 8806c18d-c71d-4306-8eb3-43a8b538b31a
eguzki commented 2 months ago

I recommend rebasing this and test this with the new openresty 1.21.4.3

tkan145 commented 1 month ago

Close in favor of #1465