Link profiles to indexers #611

rsevilla87 · 2024-03-18T21:25:21Z

Type of change

Description

Multiple changes with the goal to adopt the updated version the metricsEndpoints API in the configuration file and link metrics, alerts (supporting multiple files now) and measurements to indexers:

With this change the configuration file now looks like:

metricsEndpoints:
- prometheusURL: http://localhost:9090
  token: blablabla
  indexer: 
    type: opensearch
    esServers: ["https://es-instance.com:9200"]
    defaultIndex: kube-burner
  metrics: [metrics-profile.yaml]
  alerts: [alert-profile.yaml]
- prometheusURL: http://localhost:9090
  token: blablabla
  indexer: 
    type: local
  metrics: [metrics-profile.yaml]
  alerts: [alert-profile.yaml]

Another important feature is that now It's possible to "split" the measurement metrics and send them to different indexers thanks to the new fields timeseriesIndexer and quantilesIndexer. i.e:

metricsEndpoints:
- indexer:
    type: local
    alias: local-indexer
- indexer:
    type: opensearch
    defaultIndex: kube-burner
    esServers: ["https://opensearch.domain:9200"]
    alias: opensearch-indexer
global:
  measurements:
  - name: podLatency
    timeseriesIndexer: local-indexer
    quantilesIndexer: opensearch-indexer

With the snippet above, the measurement podLatency would use the local indexer for timeseries metrics and opensearch for the quantile metrics. cc: @afcollins

Related Tickets & Documents

Related Issue #
Closes #

cmd/kube-burner/kube-burner.go

afcollins · 2024-03-19T20:25:03Z

So timeseriesIndexer: 1 is the index ordinal of the indexers defined under endpoints starting with 1 (not 0) ?

I like being able to specify different metrics profiles for each indexer, so that then we can push report mode to elastic and capture the timeseries locally, for metrics profile metrics.

rsevilla87 · 2024-03-20T09:08:07Z

So timeseriesIndexer: 1 is the index ordinal of the indexers defined under endpoints starting with 1 (not 0) ?

I like being able to specify different metrics profiles for each indexer, so that then we can push report mode to elastic and capture the timeseries locally, for metrics profile metrics.

Yeah, that number points to the indexer to be used. The reason not to use 0 was that 0 is the "null" value of an integer variable, so in case of not specifying timeseriesIndexer 0 would be assumed. Open to other ideas though; Im not super-fan of this either.

vishnuchalla · 2024-03-25T20:51:35Z

Except for the build and test failures, overall idea looks good to me.

vishnuchalla · 2024-03-25T20:54:02Z

So timeseriesIndexer: 1 is the index ordinal of the indexers defined under endpoints starting with 1 (not 0) ?
I like being able to specify different metrics profiles for each indexer, so that then we can push report mode to elastic and capture the timeseries locally, for metrics profile metrics.

Yeah, that number points to the indexer to be used. The reason not to use 0 was that 0 is the "null" value of an integer variable, so in case of not specifying timeseriesIndexer 0 would be assumed. Open to other ideas though; Im not super-fan of this either.

Instead of numbering here, can we have a name filed in each metricsEndpoint that can be used to reference as an alias for indexer? that way I feel it will be more intuitive for an end user.

vishnuchalla · 2024-03-27T12:17:05Z

So timeseriesIndexer: 1 is the index ordinal of the indexers defined under endpoints starting with 1 (not 0) ?
I like being able to specify different metrics profiles for each indexer, so that then we can push report mode to elastic and capture the timeseries locally, for metrics profile metrics.

Yeah, that number points to the indexer to be used. The reason not to use 0 was that 0 is the "null" value of an integer variable, so in case of not specifying timeseriesIndexer 0 would be assumed. Open to other ideas though; Im not super-fan of this either.

Instead of numbering here, can we have a name filed in each metricsEndpoint that can be used to reference as an alias for indexer? that way I feel it will be more intuitive for an end user.

Few more thoughts to add on top of it.

Currently Prometheus metrics are being indexed to all the indexers. Can we have an option to restrict it to one specific indexer?
As an extension of above point I think it is good to have 3 fields for indexing, timeSeriesIndexer (for prom metrics), latenciesIndexer (for pod latencies etc) and quantilesIndexer (for pod latency quantiles) at global level (i.e outside measurements config) and at each job level. Meaning indexers at job level will override the ones at global level if a user wants to.
Default values of global indexers should be all until unless user explicitly specifies index aliases.

rsevilla87 · 2024-03-31T00:08:49Z

So timeseriesIndexer: 1 is the index ordinal of the indexers defined under endpoints starting with 1 (not 0) ?
I like being able to specify different metrics profiles for each indexer, so that then we can push report mode to elastic and capture the timeseries locally, for metrics profile metrics.

Yeah, that number points to the indexer to be used. The reason not to use 0 was that 0 is the "null" value of an integer variable, so in case of not specifying timeseriesIndexer 0 would be assumed. Open to other ideas though; Im not super-fan of this either.

Instead of numbering here, can we have a name filed in each metricsEndpoint that can be used to reference as an alias for indexer? that way I feel it will be more intuitive for an end user.

Few more thoughts to add on top of it.
* Currently Prometheus metrics are being indexed to all the indexers. Can we have an option to restrict it to one specific indexer?

They are not, the idea of this PR is to link metricsProfiles to indexers so we can restrict what we send to each indexer, if not working like this there must be a bug

rsevilla87 · 2024-04-01T09:49:51Z

So timeseriesIndexer: 1 is the index ordinal of the indexers defined under endpoints starting with 1 (not 0) ?
I like being able to specify different metrics profiles for each indexer, so that then we can push report mode to elastic and capture the timeseries locally, for metrics profile metrics.

Yeah, that number points to the indexer to be used. The reason not to use 0 was that 0 is the "null" value of an integer variable, so in case of not specifying timeseriesIndexer 0 would be assumed. Open to other ideas though; Im not super-fan of this either.

Update, I've updated the code to use indexer aliases rather than integers pointing to an indexer

vishnuchalla · 2024-04-12T23:11:26Z

pkg/util/metrics/metrics.go

+	if scraperConfig.MetricsEndpoint != "" {
+		scraperConfig.ConfigSpec.MetricsEndpoints = DecodeMetricsEndpoint(scraperConfig.MetricsEndpoint)
+	}
+	for pos, metricsEndpoint := range scraperConfig.ConfigSpec.MetricsEndpoints {


Is this loop indexing metrics.yaml in all the given list of indexers?

I don't follow you... what do you mean?

vishnuchalla · 2024-04-12T23:12:45Z

pkg/measurements/factory.go

 	globalCfg = configSpec.GlobalConfig
 	factory = measurementFactory{
 		createFuncs: make(map[string]measurement),
 		metadata:    metadata,
 	}
 	for _, measurement := range globalCfg.Measurements {
+		indexerFound = false
+		if measurement.QuantilesIndexer != "" {


Can you refactor this if-block? Looks like common logic being used for both the indexers.

updated and optimized this logic, PTAL

Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

…y are not necessary Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

vishnuchalla

lgtm

## Type of change - [ ] Refactor - [ ] New feature - [x] Bug fix - [ ] Optimization - [ ] Documentation Update ## Description #611 added `metricsEndpoints`. Update examples to fix the error. ``` level=fatal msg="Config error: error decoding configuration file: yaml: unmarshal errors:\n line 2: field indexers not found in type config.Spec" file="kube-burner.go:97" ``` ## Related Tickets & Documents - Related Issue # - Closes # Signed-off-by: Shuaiyi Zhang <zhang_syi@qq.com>

## Type of change - [ ] Refactor - [ ] New feature - [x] Bug fix - [ ] Optimization - [ ] Documentation Update ## Description My mistake, kube-burner-ocp doesn't work well with the latest changes pushed in #611 ## Related Tickets & Documents - Related Issue # - Closes # Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

rsevilla87 requested review from vishnuchalla and shashank-boyapally March 18, 2024 21:25

rsevilla87 force-pushed the indexers-profiles branch 2 times, most recently from 450edb0 to 816eade Compare March 19, 2024 08:12

rsevilla87 commented Mar 19, 2024

View reviewed changes

cmd/kube-burner/kube-burner.go Show resolved Hide resolved

rsevilla87 commented Mar 19, 2024

View reviewed changes

cmd/kube-burner/kube-burner.go Show resolved Hide resolved

rsevilla87 added documentation Improvements or additions to documentation enhancement New feature or request labels Mar 19, 2024

rsevilla87 requested review from afcollins and jtaleric March 19, 2024 11:48

rsevilla87 marked this pull request as ready for review March 19, 2024 11:48

rsevilla87 force-pushed the indexers-profiles branch from 67ad91a to 9822ac4 Compare March 25, 2024 10:02

rsevilla87 force-pushed the indexers-profiles branch 2 times, most recently from c33c96d to e4cd82f Compare April 1, 2024 10:58

vishnuchalla reviewed Apr 12, 2024

View reviewed changes

rsevilla87 force-pushed the indexers-profiles branch 2 times, most recently from dfe0d8c to 0810041 Compare April 15, 2024 10:02

rsevilla87 added 5 commits April 15, 2024 12:05

Link profiles to indexers

2ff7db3

Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

Update docs

66691c1

Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

Fix metric name typo

79e8f03

Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

Mark config as required

1bd4948

Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

Rebase

7864918

Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

rsevilla87 added 7 commits April 15, 2024 12:05

Indexer alias

53c1b83

Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

Updating docs

68c75c5

Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

Updating tests

1bfdbe7

Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

Remove all

1071455

Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

Pass configSec by refernce to overwrite metricsEndpoints

571755b

Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

Fix nil pointer reference

e3d4bad

Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

Optimize code to remove duplicated logic

03c23a4

Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

rsevilla87 force-pushed the indexers-profiles branch from 0810041 to 03c23a4 Compare April 15, 2024 10:05

rsevilla87 mentioned this pull request Apr 15, 2024

Remove default metrics and alerts in BYOWL workloads kube-burner/kube-burner-ocp#49

Merged

7 tasks

rsevilla87 added 2 commits April 17, 2024 11:07

Remove duplicated append

50b5f64

Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

Add additional condition to avoid creating PrometheusClients when the…

fc4d292

…y are not necessary Signed-off-by: Raul Sevilla <rsevilla@redhat.com>

vishnuchalla approved these changes Apr 19, 2024

View reviewed changes

rsevilla87 merged commit d0c1563 into kube-burner:main Apr 19, 2024
6 checks passed

ZhangShuaiyi mentioned this pull request Apr 22, 2024

update metricsEndpoints in examples #624

Merged

5 tasks

rsevilla87 mentioned this pull request Apr 22, 2024

Embedconfig bugfixes #625

Merged

5 tasks

afcollins mentioned this pull request Apr 25, 2024

[BUG] kube-burner#611 reverted appending UUID to local metrics dir #634

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Link profiles to indexers #611

Link profiles to indexers #611

rsevilla87 commented Mar 18, 2024 •

edited

afcollins commented Mar 19, 2024

rsevilla87 commented Mar 20, 2024

vishnuchalla commented Mar 25, 2024

vishnuchalla commented Mar 25, 2024

vishnuchalla commented Mar 27, 2024 •

edited

rsevilla87 commented Mar 31, 2024

rsevilla87 commented Apr 1, 2024

vishnuchalla Apr 12, 2024

rsevilla87 Apr 15, 2024

vishnuchalla Apr 12, 2024

rsevilla87 Apr 15, 2024

vishnuchalla left a comment

Link profiles to indexers #611

Link profiles to indexers #611

Conversation

rsevilla87 commented Mar 18, 2024 • edited

Type of change

Description

Related Tickets & Documents

afcollins commented Mar 19, 2024

rsevilla87 commented Mar 20, 2024

vishnuchalla commented Mar 25, 2024

vishnuchalla commented Mar 25, 2024

vishnuchalla commented Mar 27, 2024 • edited

rsevilla87 commented Mar 31, 2024

rsevilla87 commented Apr 1, 2024

vishnuchalla Apr 12, 2024

Choose a reason for hiding this comment

rsevilla87 Apr 15, 2024

Choose a reason for hiding this comment

vishnuchalla Apr 12, 2024

Choose a reason for hiding this comment

rsevilla87 Apr 15, 2024

Choose a reason for hiding this comment

vishnuchalla left a comment

Choose a reason for hiding this comment

rsevilla87 commented Mar 18, 2024 •

edited

vishnuchalla commented Mar 27, 2024 •

edited