Skip to content

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.

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.


@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