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

Introduce transaction_id policy #1460

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- Fixed 3scale Batcher policy unable to handle `app_id`/`access_token` contains special characters [PR #1457](https://github.com/3scale/APIcast/pull/1457) [THREESCALE-10934](https://issues.redhat.com/browse/THREESCALE-10934)

### Added

- Introduce `transaction_id` policy [PR #1460](https://github.com/3scale/APIcast/pull/1460) [THREESCALE-10973](https://issues.redhat.com/browse/THREESCALE-10973)

## [3.15.0] 2024-04-04

### Fixed
Expand Down
65 changes: 65 additions & 0 deletions gateway/src/apicast/policy/transaction_id/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# APICast Transaction-ID

## Description

When enabled this policy adds a new header with unique ID to all of the request processed by APIcast. The unique ID header can also be included in the response to the client.

If the header is empty or non-existent, this policy will generate a UUID as the value of the user-defined header name

If a header with the same name is already present in the client request or upstream response, the policy will not modify it.

## Example configuration

* Add request unique ID
```
"policy_chain": [
{
"name": "transaction_id",
"configuration": {
"header_name": "X-Transaction-ID"
},
"version": "builtin",
},
{
"name": "apicast.policy.apicast"
}
]
```

* Include the header in response
```
"policy_chain": [
{
"name": "transaction_id",
"configuration": {
"header_name": "X-Transaction-ID",
"include_in_response": true
},
"version": "builtin",
},
{
"name": "apicast.policy.apicast"
}
]
```

* Use with Logging policy

```
"policy_chain": [
{
"name": "transaction_id",
"configuration": {
"header_name": "X-Transaction-ID"
},
"version": "builtin",
},
{
"name": "apicast.policy.logging",
"configuration": {
"enable_access_logs": false,
"custom_logging": "\"{{request}}\" to service {{service.id}} and {{service.name}} with ID {{req.headers.X-Transaction-ID}}",
}
}
]
```
24 changes: 24 additions & 0 deletions gateway/src/apicast/policy/transaction_id/apicast-policy.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"$schema": "http://apicast.io/policy-v1/schema#manifest#",
"name": "Transaction ID",
"summary": "Add unique ID to the request/response header",
"description": ["This policy adds a unique ID to each request and response header proxied through APIcast",
"The policy will not add a unique ID if the request/response already has a header with the configured header_name."
],
"version": "builtin",
"configuration": {
"type": "object",
"properties": {
"header_name": {
"type": "string",
"description": "The HTTP header name to use for the transaction IDs",
"default": "X-Transaction-ID"
},
"include_in_response": {
"type": "boolean",
"description": "Whether to include the header to the response",
"default": false
}
}
}
}
1 change: 1 addition & 0 deletions gateway/src/apicast/policy/transaction_id/init.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
return require('transaction_id')
46 changes: 46 additions & 0 deletions gateway/src/apicast/policy/transaction_id/transaction_id.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
--- Transaction ID policy
-- This policy add a uniqud ID to the user defined header. This can help to identify
-- request from access log or the trace

local policy = require('apicast.policy')
local _M = policy.new('Transaction ID', 'builtin')

local uuid = require 'resty.jit-uuid'

local new = _M.new

function _M.new(config)
local self = new(config)
local conf = config or {}
self.header_name = conf.header_name
self.include_in_response = conf.include_in_response or false

return self
end

function _M:rewrite(context)
local transaction_id = ngx.req.get_headers()[self.header_name]

if not transaction_id or transaction_id == "" then
transaction_id = uuid.generate_v4()
ngx.req.set_header(self.header_name, transaction_id)
end

if self.include_in_response then
context.transaction_id = transaction_id
end
end

function _M:header_filter(context)
if not self.include_in_response then
return

Check warning on line 36 in gateway/src/apicast/policy/transaction_id/transaction_id.lua

View check run for this annotation

Codecov / codecov/patch

gateway/src/apicast/policy/transaction_id/transaction_id.lua#L36

Added line #L36 was not covered by tests
end

local transaction_id = ngx.resp.get_headers()[self.header_name]
if not transaction_id or transaction_id == "" then
transaction_id = context.transaction_id
end
ngx.header[self.header_name] = transaction_id
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: I wonder if this ngx.header[self.header_name] = transaction_id statement should only be executed in case it is not in the headers or empty.

The difference in behavior regarding what you coded is when the response header exists but it is empty. in your case, request transaction ID has preference over empty response header.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference in behavior regarding what you coded is when the response header exists but it is empty. in your case, request transaction ID has preference over empty response header.

wouldn't transaction_id == "" already covered that?

Regarding the logging, we have customer complained about extra log line, see THREESCALE-5225. I think the best solution is to inject this ID the the log, similar with what we did for the request-id

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you are right. I got confused.

The only diff would be that you are setting the value even when it is already set. Which is harmless. It is a matter of style. LGTM.

end

return _M
80 changes: 80 additions & 0 deletions spec/policy/transaction_id/transaction_id_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
local TransactionIDPolicy = require('apicast.policy.transaction_id')
local uuid = require('resty.jit-uuid')

describe('fapi_1_baseline_profile policy', function()
local ngx_req_headers = {}
local ngx_resp_headers = {}
local context = {}
before_each(function()
ngx.header = {}
ngx_req_headers = {}
ngx_resp_headers = {}
context = {}
stub(ngx.req, 'get_headers', function() return ngx_req_headers end)
stub(ngx.req, 'set_header', function(name, value) ngx_req_headers[name] = value end)
stub(ngx.resp, 'get_headers', function() return ngx_resp_headers end)
stub(ngx.resp, 'set_header', function(name, value) ngx_resp_headers[name] = value end)
end)

describe('.new', function()
it('works without configuration', function()
assert(TransactionIDPolicy.new())
end)
end)

describe('.rewrite', function()
it('do not overwrite existing header', function()
ngx_req_headers['transaction-id'] = 'abc'
local config = {header_name='transaction-id'}
local transaction_id_policy = TransactionIDPolicy.new(config)
transaction_id_policy:rewrite()
assert.same('abc', ngx.req.get_headers()['transaction-id'])
end)

it('generate uuid if header does not exist', function()
local config = {header_name='transaction-id'}
local transaction_id_policy = TransactionIDPolicy.new(config)
transaction_id_policy:rewrite()
assert.is_true(uuid.is_valid(ngx.req.get_headers()['transaction-id']))
end)

it('generate uuid if header is empty', function()
ngx_req_headers['transaction-id'] = ''
local config = {header_name='transaction-id'}
local transaction_id_policy = TransactionIDPolicy.new(config)
transaction_id_policy:rewrite()
assert.is_true(uuid.is_valid(ngx.req.get_headers()['transaction-id']))
end)
end)

describe('.header_filter', function()
it('set response transaction-id if configured', function()
ngx_req_headers['transaction-id'] = 'abc'
local config = {header_name='transaction-id', include_in_response=true}
local transaction_id_policy = TransactionIDPolicy.new(config)
transaction_id_policy:rewrite(context)
transaction_id_policy:header_filter(context)
assert.same('abc', ngx.header['transaction-id'])
end)

it('set response transaction-id if configured - uuid', function()
ngx_req_headers['transaction-id'] = ''
local config = {header_name='transaction-id', include_in_response=true}
local transaction_id_policy = TransactionIDPolicy.new(config)
transaction_id_policy:rewrite(context)
local id = ngx.req.get_headers()['transaction-id']
transaction_id_policy:header_filter(context)
assert.same(id, ngx.header['transaction-id'])
end)

it('do not override if response contain the exisitng header', function()
ngx_req_headers['transaction-id'] = 'abc'
ngx_resp_headers['transaction-id'] = 'edf'
local config = {header_name='transaction-id', include_in_response=true}
local transaction_id_policy = TransactionIDPolicy.new(config)
transaction_id_policy:rewrite(context)
transaction_id_policy:header_filter(context)
assert.same('edf', ngx.header['transaction-id'])
end)
end)
end)
Loading
Loading