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.

Sign up for GitHub

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

Jump to bottom

perf(tracing/request-id): introduce $kong_request_id #11725

Merged
merged 26 commits into from
Oct 13, 2023
Merged

perf(tracing/request-id): introduce $kong_request_id #11725

merged 26 commits into from
Oct 13, 2023

Conversation

chronolaw
Copy link
Contributor

@chronolaw chronolaw commented Oct 10, 2023 •
edited
Loading

Summary

The version of lua-kong-module has bumped to 0.8.0, introducing a new variable $kong_request_id.

KAG-2734

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Full changelog

  • [Implement ...]

Issue reference

Fix #[issue number]

@zhongweiy
Copy link
Contributor

It will be helpful to have performance data after applying this patch and https://github.com/Kong/lua-kong-nginx-module/pull/70/files.

Here is some performance data done before for reference.

  1. https://konghq.atlassian.net/browse/KAG-2692?focusedCommentId=113705
  2. nginx_config is in the description of this ticket https://konghq.atlassian.net/browse/KAG-2692 and wrk command: wrk -t1 -c40 -d10s http://127.0.0.1:80

@chronolaw chronolaw marked this pull request as ready for review October 11, 2023 00:23
@pull-request-size pull-request-size bot added size/M and removed size/S labels Oct 11, 2023
@chronolaw chronolaw requested review from dndx and ADD-SP October 11, 2023 02:26
@chronolaw chronolaw added this to the 3.5.0 milestone Oct 11, 2023
@chronolaw chronolaw requested a review from samugi October 11, 2023 06:24
@chronolaw chronolaw mentioned this pull request Oct 11, 2023
Merged
@zhongweiy
Copy link
Contributor

Shall we also update the FIPS document like "core/uuid" does? https://docs.konghq.com/gateway/latest/kong-enterprise/fips-support/

@fffonion
Copy link
Contributor

fffonion commented Oct 12, 2023 •
edited
Loading

The FIPS document is out-of-date, I will create a PR that updates it along with the request_id change.
https://github.com/Kong/docs.konghq.com/pull/6286/files

ADD-SP
ADD-SP requested changes Oct 12, 2023
View reviewed changes
Copy link
Contributor

@ADD-SP ADD-SP left a comment

Choose a reason for hiding this comment

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

Please rebase on the master and adapt to changes of #11017, that PR also uses $request_id.

cc @samugi

@fffonion fffonion mentioned this pull request Oct 12, 2023
Merged
2 tasks
@chronolaw chronolaw force-pushed the perf/introduce_var_kong_request_id branch from 0e74292 to b3843c9 Compare October 12, 2023 07:56
@chronolaw
Copy link
Contributor Author

Please rebase on the master and adapt to changes of #11017, that PR also uses $request_id.

cc @samugi

Thank you @ADD-SP , I have rebased the branch and fixed some code in #11017.

@chronolaw chronolaw requested a review from samugi October 12, 2023 08:01
@chronolaw chronolaw force-pushed the perf/introduce_var_kong_request_id branch from fe65db2 to 594d055 Compare October 13, 2023 05:37
@dndx dndx merged commit 348c980 into master Oct 13, 2023
23 checks passed
@dndx dndx deleted the perf/introduce_var_kong_request_id branch October 13, 2023 07:09
@samugi samugi mentioned this pull request Nov 9, 2023
Merged
3 tasks
samugi pushed a commit that referenced this pull request Nov 9, 2023
@chronolaw @samugi
perf(request-id): use $kong_request_id for $request_id for better…
6adb660 
… performance (#11725)

This has better performance than `$request_id`.

KAG-2734
@samugi samugi mentioned this pull request Nov 13, 2023
Merged
3 tasks
samugi pushed a commit that referenced this pull request Nov 15, 2023
@chronolaw @samugi
perf(request-id): use $kong_request_id for $request_id for better…
e29c443 
… performance (#11725)

Bumped lua-kong-nginx-module to `0.8.0`.

This has better performance than `$request_id`.

KAG-2734
samugi pushed a commit that referenced this pull request Nov 15, 2023
@chronolaw @samugi
perf(request-id): use $kong_request_id for $request_id for better…
5345aa4 
… performance (#11725)

This has better performance than `$request_id`.

KAG-2734
samugi pushed a commit that referenced this pull request Nov 15, 2023
@chronolaw @samugi
perf(request-id): use $kong_request_id for $request_id for better…
88b385b 
… performance (#11725)

Bumped lua-kong-nginx-module to `0.8.0`.

This has better performance than `$request_id`.

KAG-2734
samugi pushed a commit that referenced this pull request Nov 20, 2023
@chronolaw @samugi
perf(request-id): use $kong_request_id for $request_id for better…
6ef29b7 
… performance (#11725)

This has better performance than `$request_id`.

KAG-2734
@kikito kikito mentioned this pull request Jan 24, 2024
Closed
3 tasks
AndyZhang0707 added a commit that referenced this pull request Jul 18, 2024
@AndyZhang0707
Revert "perf(request-id): use $kong_request_id for $request_id fo…
c6c37f9 
…r better performance (#11725)"

This reverts commit 88b385b.
AndyZhang0707 added a commit that referenced this pull request Jul 26, 2024
@AndyZhang0707
Revert "perf(request-id): use $kong_request_id for $request_id fo…
7a5f4b3 
…r better performance (#11725)"

This reverts commit 88b385b.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samugi samugi samugi approved these changes

@ADD-SP ADD-SP ADD-SP approved these changes

@zhongweiy zhongweiy zhongweiy approved these changes

@dndx dndx dndx approved these changes

@chobits chobits chobits approved these changes

Assignees

@chronolaw chronolaw

Labels
core/templates core/tracing plugins/aws-lambda size/M
Projects
None yet
Milestone
3.5.0
Development

Successfully merging this pull request may close these issues.
8 participants
@chronolaw @zhongweiy @fffonion @bungle @dndx @chobits @samugi @ADD-SP