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

refactor(tracing): simplified the dynamic hooks code #12461

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

bungle
Copy link
Member

@bungle bungle commented Jan 30, 2024

Summary

Removes ugly code generation with simple local functions.

KAG-3870

@bungle bungle requested review from chronolaw and ADD-SP January 30, 2024 11:36
@bungle bungle force-pushed the perf/tracing-hooks branch 3 times, most recently from 9f3845c to 757cae6 Compare January 30, 2024 12:20
@bungle
Copy link
Member Author

bungle commented Jan 30, 2024

The non-varargs optimization can be added if neccessary. (Edit: I added small optimization for that on second commit).

ADD-SP
ADD-SP previously requested changes Jan 30, 2024
kong/dynamic_hook/init.lua Show resolved Hide resolved
@bungle bungle force-pushed the perf/tracing-hooks branch 3 times, most recently from f6d2e4a to 1aa69c0 Compare January 30, 2024 12:55
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.

Since we need to profile and benchmark this PR, could you please convert this PR as a draft?

kong/dynamic_hook/init.lua Outdated Show resolved Hide resolved
@bungle bungle force-pushed the perf/tracing-hooks branch from 1aa69c0 to 405db06 Compare January 30, 2024 12:56
@bungle bungle marked this pull request as draft January 30, 2024 12:57
@bungle
Copy link
Member Author

bungle commented Jan 30, 2024

Since we need to profile and benchmark this PR, could you please convert this PR as a draft?

Converted to draft.

@bungle bungle force-pushed the perf/tracing-hooks branch 4 times, most recently from 91b3278 to da9c200 Compare January 30, 2024 15:07
@bungle bungle dismissed ADD-SP’s stale review January 31, 2024 15:09

changes made.

@bungle bungle force-pushed the perf/tracing-hooks branch 2 times, most recently from 36f1109 to 6bb8e66 Compare February 16, 2024 09:44
@ADD-SP ADD-SP force-pushed the perf/tracing-hooks branch from 6bb8e66 to 4db90f5 Compare February 20, 2024 02:21
@ADD-SP
Copy link
Contributor

ADD-SP commented Feb 20, 2024

Rebased on master.

kong/dynamic_hook/init.lua Outdated Show resolved Hide resolved
@bungle bungle added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Feb 26, 2024
@bungle bungle force-pushed the perf/tracing-hooks branch 2 times, most recently from ab85eeb to 2c24ef1 Compare February 27, 2024 11:15
@bungle bungle force-pushed the perf/tracing-hooks branch 2 times, most recently from c43fb35 to c0594ee Compare February 29, 2024 09:08
@bungle bungle changed the title perf(tracing): simplified the dynamic hooks code refactor(tracing): simplified the dynamic hooks code Feb 29, 2024
@bungle bungle marked this pull request as ready for review February 29, 2024 09:11
@bungle bungle removed the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Feb 29, 2024
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.

Don't forget to remove the first argument at the definition of run_hooks()

req_dyn_hook_run_hooks(ctx, "timing", "before:plugin_iterator")

@bungle
Copy link
Member Author

bungle commented Mar 5, 2024

Don't forget to remove the first argument at the definition of run_hooks()

req_dyn_hook_run_hooks(ctx, "timing", "before:plugin_iterator")

I have removed the ctx from run_hooks.

@bungle bungle added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Mar 5, 2024
@ADD-SP ADD-SP force-pushed the perf/tracing-hooks branch from d0d75dd to 7ca1742 Compare March 5, 2024 07:48
@bungle bungle merged commit 336849d into master Mar 5, 2024
28 checks passed
@bungle bungle deleted the perf/tracing-hooks branch March 5, 2024 09:46
@team-gateway-bot
Copy link
Collaborator

Cherry-pick failed for master, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git remote add upstream https://github.com/kong/kong-ee
git fetch upstream master
git worktree add -d .worktree/cherry-pick-12461-to-master-to-upstream upstream/master
cd .worktree/cherry-pick-12461-to-master-to-upstream
git checkout -b cherry-pick-12461-to-master-to-upstream
ancref=$(git merge-base 29285c3867038b66a57591ae09b640f92c35c4a0 7ca1742dcd1a2113a3e8a7e9be833fa7c6a16895)
git cherry-pick -x $ancref..7ca1742dcd1a2113a3e8a7e9be833fa7c6a16895

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/proxy size/XL skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants