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

Metrics #24

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

Athishpranav2003
Copy link

@Athishpranav2003 Athishpranav2003 commented Jun 14, 2024

Support to add prometheus metrics for observability for throttling of logs as requested here #15 . Opens more scope to add lot more metrics related to throttling. Corresponding tests are also added to the testing script and verified. Few plugin versions have been bumped up inorder to avoid few build issues.

@@ -1 +1,3 @@
*.gem
nclude-dependencies

Choose a reason for hiding this comment

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

can you fix this?

Copy link
Author

Choose a reason for hiding this comment

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

In general the lock file is not included in the plugins(since it depends on build env). Thats why i didnt include. I checked with other plugins too. Its the same

desc <<~DESC
Whether to emit a metric when the rate limit is exceeded. The metric is fluentd_throttle_rate_limit_exceeded
DESC
config_param :group_emit_metrics, :bool, :default => false

Choose a reason for hiding this comment

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

Can we keep the name simple?
just enable_metrics should be fine.

@@ -68,12 +83,26 @@ def configure(conf)

raise "group_warning_delay_s must be >= 1" \
unless @group_warning_delay_s >= 1

hostname = Socket.gethostname
Copy link

@dipendra-singh dipendra-singh Jun 25, 2024

Choose a reason for hiding this comment

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

Put the next few lines under the check if metrics enabled.

end

def start
super

@counters = {}
# TODO add more relevant metrics to throttling
@metrics = {throttle_rate_limit_exceeded: get_counter(:fluentd_throttle_rate_limit_exceeded, "The exceeded rate of pods in the group")}

Choose a reason for hiding this comment

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

this can be a variable. I dont think we need a map here

Copy link
Author

Choose a reason for hiding this comment

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

This was made as a map just to make it standard to add more metrics. It can act as a template to add more metrics by adding in the map

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