Prevent indefinite unsent metrics loop in outpus.azure_monitor #15908
Comments
Hr0bar
added
the
feature request
|
Actually just found #14928 where it says: While in code comments we see this: Im not so sure anymore that the workaround with aggressive buffer limit will help now, since its not very clear whether old are dropped, or old kept and new dropped, in which case aggressive buffer limit wont help. |
|
The code comment is correct and the spec needs correction. Are you willing to put up a PR to fix the spec wording? I can confirm that actually the oldest metrics in the buffer are dropped (overwritten by new metrics) first. |
|
Regarding your issue, I'm preparing a new way of selectively dropping metrics on outputs and I want to hold this up until this is ready. Is that OK for you? |
|
Thanks, yes we are fine waiting for some more comprehensive solution. Ill have a look at PR for wording in the spec (and possibly also expanding the wording in "metric_buffer_limit" description in docs, perhaps even azure output plugin docs could use some mention of the Azure limit, some Azure users may not be aware). |
|
Well thats one way to solve it - by dropping all rejected metrics only, that would certainly work for this use case as well (rather than relying on time based drop). But... I would assume this could be difficult to generalize for many output plugins, as I imagine some external systems (in this case Azure) reject whole batch, some provide info what specifically they rejected and why, some dont etc. No idea how Azure monitor output behaves for example... is it sent one by one metric? or full batch? What happens when only one metric in full batch is rejected ? Do we drop whole batch (dont think so)? How do we know which one caused the reject by service error if its only one out of many? |
|
Did you check the spec? It extends the current batch handling to be able to do exactly this, drop specific metrics of a batch or drop the whole batch or do whatever the output requires. Currently, you can either accept a batch (removing it from the buffer) or keep the batch (requeueing the whole thing on next write)... |
|
Yes, understood that, and yes that would be a working solution :) I was just thinking about how that solution can be achieved as someone with no understanding of how it currently works ( I was under the impression that now whole batch is sent as a single payload to output destination - and theorizing whether if that would be the case if it needs to be split and sent metric by metric instead to output destination, to know exactly which metrics get rejected one by one so we can drop only specific metrics instead of whole batch) |
|
@Hr0bar I'm looking into this issue currently and I wonder how we know what the filter time should be? In your error message the valid time-range is 30 min ago and 4 min in the future but I cannot find that anywhere in the Azure documentation. Should this be a config option? |
|
Found this:
https://learn.microsoft.com/en-us/azure/azure-monitor/essentials/metrics-store-custom-rest-api
"Each data point sent to Azure Monitor must be marked with a timestamp.
This timestamp captures the date and time at which the metric value is
measured or collected. Azure Monitor accepts metric data with timestamps as
far as 20 minutes in the past and 5 minutes in the future. The timestamp
must be in ISO 8601 format."
So seems inconsistent on Microsoft side.
…On Wed, Nov 6, 2024, 18:31 Sven Rebhan ***@***.***> wrote:
@Hr0bar <https://github.com/Hr0bar> I'm looking into this issue currently
and I wonder how we know what the filter time should be? In your error
message the valid time-range is 30 min ago and 4 min in the future but I
cannot find that anywhere in the Azure documentation. Should this be a
config option?
—
Reply to this email directly, view it on GitHub
<#15908 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABTNGZWHJDWD5W24PW4XKBTZ7JG6FAVCNFSM6AAAAABOLD3U46VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRQGM4DANJUGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
So I think making this a config option is the right thing to do then with 20 min in the past and 4 min in the future (i.e. the intersection of both time-sets ;-)). Parsing this from the response might be another option but I do worry that this will be a fragile foot-gun... |
|
Agree, its likely not stable on Azure side (both the values & possible error msg string), so configurable with conservative intersection default makes sense! |
|
Thanks! I can try the binary, but dont think I can simulate the Azure (or network proxy etc) outages reliably to reproduce the issue for test. Since I filled this issue we experienced the issue zero times, only once Azure was Timeouting for about 45 minutes, but since some batches (about every tenth) went through and reduced the backlog, it did not trigger the cyclic infinite unsent metrics even then. This would likely be best quick tested with some mock setup with different endpoint than Azure perhaps. there is one typo in the PR docs "witin" should be "within" I also see the 30 min more relaxed limit was chosen as default for past metrics, perhaps we could find out some official answer from Microsoft, instead of guessing whether the limit in returned error msg is correct or the one in their documentation. But I would trust the error msg more than docs indeed. Anyway, the binary starts but fails after loading azure monitor plugin start for us: |
|
Pushed a fix for the panic above. Sorry for not getting it right in the first place... For simulation of the issue, you could simply increase the |
|
I cannot trigger it with the old version (without this PR) and file/tail plugin, seems the plugin already filters too old metrics and drops them (doesnt aggregate), and the issue only happens when the plugin accepts them and aggregates them, but fails to write, then retries sending them after too long time ? Tested the same with new (fixed panic) PR, same thing happens. Too old metrics arent even tried to be sent to Azure. Only current metrics are aggregated and sent (to output buffer). Seems there were some protections already in place. So to test, I need to provide CURRENT timestamps on imput, simulate write error/timeout for long enough duration that the CURRENT timestamps become too OLD I think. Otherwise the protections in place will refuse them in the very beginning. Correct me if I understood it wrongly. |
|
ahh, Ill try setting the timestamp_limit_past with new PR for like 1000 days in the past, and try it with that, did not realize new PR could modify the existing protections too, so they are not triggered, will try in a sec |
|
Okay, I get the:
when I set timestamp_limit_past to too old, and provide too old metrics on input. It doesnt happen when set to default 30 minutes. But its filtered out by the logic that was already present - old metrics arent even attempted to be sent. So not sure if this test helps in any way :/ As the same would happen in the old versions. And issue is only triggered with the network issues mentioned above (not filtered straight away, but buffered for retry until too much time passes). Is there a way where I can load current timestamps into output buffer, so the plugin doesnt filter them straight away, but only send them after 30 minutes? With some high flush_interval or similar ? |
|
@Hr0bar well the problem is the following: Previously, the metrics are aggregated and the aggregates are filtered by the time limit just before being pushed to the buffer. I.e. aggregated metrics older than 30 minutes are dropped. Metrics newer than 1 minute in the past were held back. All aggregated metrics within the previous timespan are added to the buffer and are written. If that write fails (e.g. due to a connectivity issue) the metrics are still in the buffer and Telegraf will attempt to write them again in the next flush cycle because write returned an error. My change does two things. It will check the metric before each write! Therefore, the 31 minute old metric will be rejected and in turn dropped from the buffer. However, there is still a (small) chance of getting a For testing, there are two cases I would be interested in:
For the second test I can write a unit-test (and actually have done so). But the first test can only be performed by you as it involves the actual Azure service... So please
|
|
Does this work? Did what you said I hope, with oneshot run and for 2 interval cycles in daemon mode as well: |
|
Seems to work! Thanks! |
Use Case
Hi, we are using outputs.azure_monitor for more than 3 years now and only now we ran into this issue which triggered this feature request. Basically a global option to drop unsent metrics by a time limit instead of buffer size limit could be a solution, or output plugin specific solution could be added?
The issue we experienced:
For more than 30 minutes the Azure endpoint was Timeouting:
Then Azure endpoint became responsive again, but since it was down for more than 30 minutes, all outputs Telegraf has buffered are now rejected by Azure, resulting in indefinite loop of never sent metrics:
...Forever until Telegraf restarts
For now we will drastically reduce the buffer size limit to avoid this. But wee add/remove metrics to monitoring all the time, its not feasible to recalculate proper buffer limits (metric_batch_size and metric_buffer_limit) every time to achieve desired unsent metrics time limit before dropping them, so this solution doesnt seem very "clean" ?
One extra note on documentation, we had to take a look at the Telegraf source code comments to find out an important info that oldest metrics are overwritten, which is important for us to know that reducing the buffer size will help us in most cases:
The official docs say just:
metric_buffer_limit: Maximum number of unwritten metrics per output. Increasing this value allows for longer periods of output downtime without dropping metrics at the cost of higher maximum memory usage.Which is not enough to make an educated configuration decision, perhaps it could be made more verbose ?
Expected behavior
Reliably configure to drop unsent metrics by time instead of buffer sizes. We add/remove metrics to monitoring all the time, its not feasible to recalculate proper buffer limits (metric_batch_size and metric_buffer_limit) every time to achieve desired unsent metrics time limit before dropping them.
Alternatively, a force drop of old metrics could occur when
E! [agent] Error writing to outputs.azure_monitor: failed to write batch: [400] 400 Bad Request: {"error":{"code":"BadRequest","message":"'time' should not be older than 30 minutes and not more than 4 minutes in the future\r\n"}}is detected in Azure output plugin, as a one plugin only solution.
Alternatively, perhaps a documentation update in Azure output plugin could be added and be enough, warning all users that aggressive buffer sizes should be used to avoid this issue. The rationale is that a global option to drop by time limit could be an overkill / not warranted enough, but that is for the Telegraf project to decide :)
Actual behavior
Azure metrics / possibly other output destinations will reject metrics older than certain time limit. During network/other issues resulting into outputs not being sent, Telegraf will buffer metrics older than that time limit indefinitely in a loop where metrics older than the time limit are scheduled to be sent (at the top of the stack) indefinitely.
Additional info
Similar to #13273
The text was updated successfully, but these errors were encountered: