Rename indexer metrics related to get operations #853

mrodm · 2022-07-26T16:23:47Z

Rename indexer get metrics (histogram) to be specific to each type of indexer.
According to the current indexers, this refers to be able to split Get operations depending on the file system, zip and storage indexers.

Calculations of these metrics have been moved to their specific implementations (Get method).

Relates #797

Before this PR, the metrics exposed for Get operations were:

# HELP epr_storage_indexer_get_duration_seconds A histogram of latencies for get processes run by the indexer.
# TYPE epr_storage_indexer_get_duration_seconds histogram
epr_storage_indexer_get_duration_seconds_bucket{le="0.005"} 90
epr_storage_indexer_get_duration_seconds_bucket{le="0.01"} 90
epr_storage_indexer_get_duration_seconds_bucket{le="0.025"} 90
epr_storage_indexer_get_duration_seconds_bucket{le="0.05"} 90
epr_storage_indexer_get_duration_seconds_bucket{le="0.1"} 90
epr_storage_indexer_get_duration_seconds_bucket{le="0.25"} 90
epr_storage_indexer_get_duration_seconds_bucket{le="0.5"} 90
epr_storage_indexer_get_duration_seconds_bucket{le="1"} 90
epr_storage_indexer_get_duration_seconds_bucket{le="2.5"} 90
epr_storage_indexer_get_duration_seconds_bucket{le="5"} 90
epr_storage_indexer_get_duration_seconds_bucket{le="10"} 90
epr_storage_indexer_get_duration_seconds_bucket{le="+Inf"} 90
epr_storage_indexer_get_duration_seconds_sum 2.2363000000000006e-05
epr_storage_indexer_get_duration_seconds_count 90

With this patch proposed, the metrics exposed will be like this:

# HELP epr_indexer_get_duration_seconds A histogram of latencies for get processes run by the indexer.
# TYPE epr_indexer_get_duration_seconds histogram
epr_indexer_get_duration_seconds_bucket{indexer="FileSystemIndexer",le="0.005"} 161
epr_indexer_get_duration_seconds_bucket{indexer="FileSystemIndexer",le="0.01"} 161
epr_indexer_get_duration_seconds_bucket{indexer="FileSystemIndexer",le="0.025"} 161
epr_indexer_get_duration_seconds_bucket{indexer="FileSystemIndexer",le="0.05"} 161
epr_indexer_get_duration_seconds_bucket{indexer="FileSystemIndexer",le="0.1"} 161
epr_indexer_get_duration_seconds_bucket{indexer="FileSystemIndexer",le="0.25"} 161
epr_indexer_get_duration_seconds_bucket{indexer="FileSystemIndexer",le="0.5"} 161
epr_indexer_get_duration_seconds_bucket{indexer="FileSystemIndexer",le="1"} 161
epr_indexer_get_duration_seconds_bucket{indexer="FileSystemIndexer",le="2.5"} 161
epr_indexer_get_duration_seconds_bucket{indexer="FileSystemIndexer",le="5"} 161
epr_indexer_get_duration_seconds_bucket{indexer="FileSystemIndexer",le="10"} 161
epr_indexer_get_duration_seconds_bucket{indexer="FileSystemIndexer",le="+Inf"} 161
epr_indexer_get_duration_seconds_sum{indexer="FileSystemIndexer"} 0.001622019999999999
epr_indexer_get_duration_seconds_count{indexer="FileSystemIndexer"} 161
epr_indexer_get_duration_seconds_bucket{indexer="ZipFileSystemIndexer",le="0.005"} 161
epr_indexer_get_duration_seconds_bucket{indexer="ZipFileSystemIndexer",le="0.01"} 161
epr_indexer_get_duration_seconds_bucket{indexer="ZipFileSystemIndexer",le="0.025"} 161
epr_indexer_get_duration_seconds_bucket{indexer="ZipFileSystemIndexer",le="0.05"} 161
epr_indexer_get_duration_seconds_bucket{indexer="ZipFileSystemIndexer",le="0.1"} 161
epr_indexer_get_duration_seconds_bucket{indexer="ZipFileSystemIndexer",le="0.25"} 161
epr_indexer_get_duration_seconds_bucket{indexer="ZipFileSystemIndexer",le="0.5"} 161
epr_indexer_get_duration_seconds_bucket{indexer="ZipFileSystemIndexer",le="1"} 161
epr_indexer_get_duration_seconds_bucket{indexer="ZipFileSystemIndexer",le="2.5"} 161
epr_indexer_get_duration_seconds_bucket{indexer="ZipFileSystemIndexer",le="5"} 161
epr_indexer_get_duration_seconds_bucket{indexer="ZipFileSystemIndexer",le="10"} 161
epr_indexer_get_duration_seconds_bucket{indexer="ZipFileSystemIndexer",le="+Inf"} 161
epr_indexer_get_duration_seconds_sum{indexer="ZipFileSystemIndexer"} 0.0005851810000000002
epr_indexer_get_duration_seconds_count{indexer="ZipFileSystemIndexer"} 161

Rename indexer get metrics (histogram) to be specific to each type of indexer. Currently, file system, zip and storage indexers.

elasticmachine · 2022-07-26T16:30:04Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-07-27T15:09:15.977+0000
Duration: 5 min 22 sec

Test stats 🧪

Test	Results
Failed	0
Passed	213
Skipped	0
Total	213

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

jsoriano

Thanks!

jsoriano · 2022-07-26T16:40:09Z

metrics/metrics.go

 		},
 	)

-	StorageIndexerGetDurationSeconds = prometheus.NewHistogram(
+	IndexerGetDurationSeconds = prometheus.NewHistogramVec(


Umm, I am wondering how useful is this metric. Looking at the sample metrics in the description, they seem to be all quite fast as these operations happen mostly in memory, and they end up all in the lower bucket.
We also have metrics in the middleware for /search, that may be more useful for final users, as it includes the time of the whole response.

So I wonder if maybe we can remove this metric. Or if we leave it, lower the boundaries of each bucket so they don't end up all in the lower bucket.

Currently I don't have data about these get operations in the storage indexer.
I was expecting that this metric would be more significant in that storage indexer than in the file system indexers. I was thinking that maybe those get ops against storage indexer would be slower (just hypothesis, I don't have data about this).

Well, current implementations are only filtering a list in memory. But it is true that the operation in the storage indexer is in a mutex whose lock can have some wait time while the index is updated. We may revisit the bucket thresholds when we have more data.

mrodm · 2022-07-26T16:40:56Z

main.go

@@ -68,7 +68,7 @@ var (
 func init() {
 	flag.BoolVar(&printVersionInfo, "version", false, "Print Elastic Package Registry version")
 	flag.StringVar(&address, "address", "localhost:8080", "Address of the package-registry service.")
-	flag.StringVar(&metricsAddress, "metrics-address", "", "Address to expose the Prometheus metrics.")
+	flag.StringVar(&metricsAddress, "metrics-address", "", "Address to expose the Prometheus metrics (experimental). ")


Just a minor change here to add this Prometheus metric endpoint is subject to changes.
Do you think it is worthy to add it ?

Sounds good to me, we may redefine something else while we start using it.

mrodm · 2022-07-26T16:41:28Z

CHANGELOG.md

+* Rename indexer metrics related to get operations. [#853](https://github.com/elastic/package-registry/pull/853)
+


Should it be moved to another section in the changelog ?

I think it can fit as a bugfix 🙂 Though it could be also seen as a breaking change.

A bugfix is fine :)

mtojek · 2022-07-27T07:35:40Z

CHANGELOG.md

+* Rename indexer metrics related to get operations. [#853](https://github.com/elastic/package-registry/pull/853)
+


A bugfix is fine :)

mtojek · 2022-07-27T07:37:37Z

storage/indexer.go

@@ -43,6 +46,7 @@ func NewIndexer(storageClient *storage.Client, options IndexerOptions) *Indexer
 	return &Indexer{
 		storageClient: storageClient,
 		options:       options,
+		label:         "StorageIndexer",


Hm.. could it be a const instead of a structure field?

I wanted to follow the same structure that it is used for FileSystemIndexers (link) where the label is part of the struct. But it is true that for storage indexer it is not needed to add that label as a field struct. So, I'll move it to be a const.

jsoriano · 2022-07-27T09:59:35Z

metrics/metrics.go

 		},
 	)

-	StorageIndexerGetDurationSeconds = prometheus.NewHistogram(
+	IndexerGetDurationSeconds = prometheus.NewHistogramVec(


Well, current implementations are only filtering a list in memory. But it is true that the operation in the storage indexer is in a mutex whose lock can have some wait time while the index is updated. We may revisit the bucket thresholds when we have more data.

mrodm · 2022-07-27T12:14:20Z

indexer.go

@@ -48,7 +44,7 @@ func (c CombinedIndexer) Get(ctx context.Context, opts *packages.GetOptions) (pa
 		packages = packages.Join(p)
 	}

-	if !opts.Filter.AllVersions {
+	if opts != nil && opts.Filter != nil && !opts.Filter.AllVersions {


This change was needed after updating this branch with main.
This Get method is called with opts as nil from https://github.com/elastic/package-registry/blob/main/main.go#L290

…s (get operations) (#3863) Update field mappings, ingest pipeline and dashboard accordingly with the changes done in elastic/package-registry#853 Specially, get operations related to indexers are now reporting the values per each indexer (using labels).

Rename indexer metrics related to get operations

02fb0ad

Rename indexer get metrics (histogram) to be specific to each type of indexer. Currently, file system, zip and storage indexers.

mrodm added the Team:Ecosystem Label for the Packages Ecosystem team label Jul 26, 2022

mrodm self-assigned this Jul 26, 2022

Update Changelog

6d32e1a

jsoriano reviewed Jul 26, 2022

View reviewed changes

mrodm commented Jul 26, 2022

View reviewed changes

mrodm requested a review from a team July 26, 2022 16:42

mrodm marked this pull request as ready for review July 26, 2022 16:46

mrodm requested a review from jsoriano July 26, 2022 16:46

mtojek approved these changes Jul 27, 2022

View reviewed changes

jsoriano approved these changes Jul 27, 2022

View reviewed changes

mrodm added 4 commits July 27, 2022 13:11

Merge upstream main branch into update_storage_metrics branch

da35579

Fix opts filter check

b56f239

Moved to a const label string in storage indexer

85fae6d

Update changelog

7d64c94

mrodm requested a review from mtojek July 27, 2022 12:11

mtojek approved these changes Jul 27, 2022

View reviewed changes

mrodm commented Jul 27, 2022

View reviewed changes

mrodm added 2 commits July 27, 2022 16:13

Update help messages

8e810cb

Replace hardcoded strings by const variables

7d15558

mrodm requested a review from jsoriano July 27, 2022 15:12

jsoriano approved these changes Jul 27, 2022

View reviewed changes

mrodm merged commit a69ff8a into elastic:main Jul 27, 2022

mrodm deleted the update_storage_metrics branch July 27, 2022 16:02

mrodm mentioned this pull request Jul 27, 2022

Update Elastic Package Registry integration to use new indexer metrics (get operations) elastic/integrations#3863

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename indexer metrics related to get operations #853

Rename indexer metrics related to get operations #853

mrodm commented Jul 26, 2022

elasticmachine commented Jul 26, 2022 •

edited

Loading

Build stats

Test stats 🧪

jsoriano left a comment

jsoriano Jul 26, 2022

mrodm Jul 26, 2022

jsoriano Jul 27, 2022

mrodm Jul 26, 2022

jsoriano Jul 26, 2022

mrodm Jul 26, 2022

jsoriano Jul 26, 2022

mtojek Jul 27, 2022

mtojek Jul 27, 2022

mtojek Jul 27, 2022

mrodm Jul 27, 2022 •

edited

Loading

jsoriano Jul 27, 2022

mrodm Jul 27, 2022

		* Rename indexer metrics related to get operations. [#853](https://github.com/elastic/package-registry/pull/853)

Rename indexer metrics related to get operations #853

Rename indexer metrics related to get operations #853

Conversation

mrodm commented Jul 26, 2022

elasticmachine commented Jul 26, 2022 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

jsoriano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrodm Jul 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Jul 26, 2022 •

edited

Loading

mrodm Jul 27, 2022 •

edited

Loading