fix: enforced max limit on metrics transmission config#2511
fix: enforced max limit on metrics transmission config#2511abhilash-sivan merged 3 commits intomainfrom
Conversation
| * @returns {*} | ||
| */ | ||
| function normalizeSingleValue(configValue, defaultValue, configPath, envVarKey) { | ||
| function normalizeSingleValue(configValue, defaultValue, configPath, envVarKey, minValue, maxValue) { |
There was a problem hiding this comment.
Since INSTANA_METRICS_TRANSMISSION_DELAY normalization uses the shared normalizeSingleValue function, the min/max validation logic has been added.
Even though the minimum value isn’t currently enforced or used, it’s included for consistency with the common normalisation approach.
| 'config.metrics.transmissionDelay', | ||
| 'INSTANA_METRICS_TRANSMISSION_DELAY' | ||
| 'INSTANA_METRICS_TRANSMISSION_DELAY', | ||
| null, |
There was a problem hiding this comment.
qs: why null passed as min value? what if customer configure 500ms? It still works?
| * @returns {*} | ||
| */ | ||
| function normalizeSingleValue(configValue, defaultValue, configPath, envVarKey) { | ||
| function normalizeSingleValue(configValue, defaultValue, configPath, envVarKey, minValue, maxValue) { |
There was a problem hiding this comment.
Please revert this and handle the check only in the normalizeMetricsConfig fn
| logger.warn( | ||
| `The value of config.metrics.transmissionDelay (or INSTANA_METRICS_TRANSMISSION_DELAY) (${config.metrics.transmissionDelay}) exceeds the maximum allowed value of 5000. Assuming the default value ${defaults.metrics.transmissionDelay}.` | ||
| ); | ||
| config.metrics.transmissionDelay = defaults.metrics.transmissionDelay; |
There was a problem hiding this comment.
This should be 5000, right?
There was a problem hiding this comment.
The default is still 1000 as per the discussions
There was a problem hiding this comment.
But if a customer sets already >5s, we should set the MAX limit?
There was a problem hiding this comment.
In this case, it should be 5000ms, that is the max allowed limit.
There was a problem hiding this comment.
ah that makes sense
abhilash-sivan
changed the title
abhilash-sivan
force-pushed
the
fix-limit-poll-rate
branch
from
6df756e to
a49dbbf
Compare
|
3 participants
Uh oh!
There was an error while loading. Please reload this page.