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

Setting Prometheus Remote_write Period default value to 1m #38553

Merged
merged 25 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
22f93e2
Correcting period for Prometheus remote_write
gizas Mar 22, 2024
fc75d6c
Merge branch 'main' of github.com:elastic/beats
gizas Mar 22, 2024
102f805
Correcting period for Prometheus remote_write
gizas Mar 22, 2024
d905adf
Adding changelog
gizas Mar 22, 2024
f0ec4f7
Updating metricbeat.reference.yaml file
gizas Mar 22, 2024
0517a07
Update x-pack/metricbeat/metricbeat.reference.yml
gizas Mar 27, 2024
cd5056b
Update x-pack/metricbeat/module/prometheus/remote_write/data.go
gizas Mar 27, 2024
f5c20ab
Update CHANGELOG.next.asciidoc
gizas Mar 27, 2024
c0e4a0c
Merge branch 'main' into remote_write_ratefix
gizas Mar 27, 2024
89369c7
Correcting typos and referring to correct variable
gizas Mar 28, 2024
300b1f7
Fixing conflicts
gizas Mar 28, 2024
cbc2d4c
Adding default value to config period of remote_write
gizas Mar 28, 2024
0802621
Adding default value to config period of remote_write
gizas Mar 28, 2024
11831b4
Removing spaces from doc
gizas Mar 28, 2024
3fbe7c0
Merge branch 'main' into remote_write_ratefix
gizas Mar 28, 2024
4cfcf01
Moving check functions to validate inside config
gizas Mar 29, 2024
f82c624
Moving check functions to validate inside config
gizas Mar 29, 2024
0cd2e64
Merge branch 'main' into remote_write_ratefix
gizas Mar 29, 2024
d1d18f4
Updating reference file
gizas Mar 29, 2024
6ae51f8
After running make check
gizas Mar 29, 2024
689422c
Merge branch 'main' into remote_write_ratefix
gizas Apr 9, 2024
a696943
Merge branch 'main' into remote_write_ratefix
gizas Apr 10, 2024
cbf265f
Merge branch 'main' of github.com:elastic/beats into remote_write_rat…
gizas Apr 10, 2024
b1b674a
Merge branch 'remote_write_ratefix' of github.com:elastic/beats into …
gizas Apr 10, 2024
d709ca0
Merge branch 'main' into remote_write_ratefix
gizas Apr 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff]
*Heartbeat*

*Metricbeat*

- Setting period for counter cache for Prometheus remote_write at least to 60sec {pull}38553[38553]

*Osquerybeat*

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ metricbeat.modules:
metricsets: ["remote_write"]
host: "localhost"
port: "9201"
use_types: true
rate_counters: true
period: 60s
-------------------------------------------------------------------------------------

`use_types` parameter (default: false) enables a different layout for metrics storage, leveraging Elasticsearch
Expand All @@ -95,6 +98,10 @@ types, including https://www.elastic.co/guide/en/elasticsearch/reference/current
the counter increment since the last collection. This metric should make some aggregations easier and with better
performance. This parameter can only be enabled in combination with `use_types`.

`period` parameter (default: 60s) configures the timeout of internal cache, which stores counter values in order to calculate rates between consecutive fetches. The parameter will be validated and all values lower than 60sec will be reset to the default value.

Note that by default prometheus pushes data with the interval of 60s (in remote write). In case that prometheus push rate is changed, the `period` parameter needs to be configured accordingly.

When `use_types` and `rate_counters` are enabled, metrics are stored like this:

[source,json]
Expand Down
17 changes: 15 additions & 2 deletions x-pack/metricbeat/module/prometheus/remote_write/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@

package remote_write

import "errors"
import (
"errors"
"time"
)

type config struct {
UseTypes bool `config:"use_types"`
RateCounters bool `config:"rate_counters"`
TypesPatterns TypesPatterns `config:"types_patterns" yaml:"types_patterns,omitempty"`
Period time.Duration `config:"period" validate:"positive"`
}

type TypesPatterns struct {
Expand All @@ -21,12 +25,21 @@ var defaultConfig = config{
TypesPatterns: TypesPatterns{
CounterPatterns: nil,
HistogramPatterns: nil},
Period: time.Second * 60,
}

func (c *config) Validate() error {
Copy link
Contributor

@tetianakravchenko tetianakravchenko Mar 28, 2024

Choose a reason for hiding this comment

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

could this function be a better fit for the period validation? check can be moved here with the info log message regarding the min value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved this to the Validate() 4cfcf01

if c.RateCounters && !c.UseTypes {
return errors.New("'rate_counters' can only be enabled when `use_types` is also enabled")
}

duration, err := time.ParseDuration(c.Period.String())
{
if err != nil {
return err
} else if duration < 60*time.Second {
// by default prometheus push data with the interval 60s, in order to calculate counter rate we are setting Period to 60secs accordingly
c.Period = time.Second * 60
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we log a warning saying the period needs to be at least 60s?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would say for now that debug log here and the comment here are enough. Afterall this is an internal cache so not many users will want to change it

}
}
return nil
}
3 changes: 2 additions & 1 deletion x-pack/metricbeat/module/prometheus/remote_write/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ func remoteWriteEventsGeneratorFactory(base mb.BaseMetricSet) (remote_write.Remo
}

if config.UseTypes {
logp.Debug("prometheus.remote_write.cache", "Period for counter cache for remote_write: %v", config.Period.String())
// use a counter cache with a timeout of 5x the period, as a safe value
// to make sure that all counters are available between fetches
counters := collector.NewCounterCache(base.Module().Config().Period * 5)
counters := collector.NewCounterCache(config.Period * 5)

g := remoteWriteTypedGenerator{
counterCache: counters,
Expand Down
Loading