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

Feat/prometheus #3547

Closed
wants to merge 2 commits into from
Closed

Feat/prometheus #3547

wants to merge 2 commits into from

Conversation

hbagdi
Copy link
Member

@hbagdi hbagdi commented Jun 15, 2018

No description provided.

@hbagdi hbagdi added pr/wip A work in progress PR opened to receive feedback pr/do not merge labels Jun 15, 2018
Prometheus plugin needs a dedicated shm to store metrics of Kong and
it's upstream. This patch sets the injects the requried shm into NGINX
template if the plugin is available for use.
The size chosen for the shm is 5 MB, which is more than
sufficient to store metrics related to a few hundred services (tested
for upto 200 services).

This is a temporary workaround until there is an API exposed from the
core which allows specifying custom shared dicts for plugin.
When such an API is introduced, the changes introduced by commit
should be reverted.
@hbagdi hbagdi force-pushed the feat/prometheus branch from e7a357b to e38b885 Compare June 15, 2018 19:35
@hbagdi hbagdi requested a review from thibaultcha June 15, 2018 20:08
@hbagdi hbagdi removed pr/do not merge pr/wip A work in progress PR opened to receive feedback labels Jun 15, 2018

if shm_value then
if not string.match(shm_value, "prometheus_metrics") then
shm_value = shm_value .. "; lua_shared_dict prometheus_metrics 5m"
Copy link
Member

Choose a reason for hiding this comment

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

This is making me regret that the loop injected in the template is not iterating over an array, which would be future proof, would not make this hack necessary, and would yield predictable results if we ever do or allow some sorting of the directives...

Copy link
Member Author

Choose a reason for hiding this comment

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

I see, good point since this the directive injection can be used as frequent internally in the code as an end user would want to do it via kong.conf.

thibaultcha pushed a commit that referenced this pull request Jun 16, 2018
thibaultcha pushed a commit that referenced this pull request Jun 16, 2018
Prometheus plugin needs a dedicated shm to store metrics of Kong and
it's upstream. This patch sets the injects the requried shm into NGINX
template if the plugin is available for use.  The size chosen for the
shm is 5 MB, which is more than sufficient to store metrics related to a
few hundred services (tested for upto 200 services).

This is a temporary workaround until there is an API exposed from the
core which allows specifying custom shared dicts for plugin.  When such
an API is introduced, the changes introduced by commit should be
reverted.

From #3547
@thibaultcha
Copy link
Member

Merged! Thank you

@thibaultcha thibaultcha deleted the feat/prometheus branch June 16, 2018 03:09
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.

2 participants