Skip to content

ES-10037 Add new write load metrics to IndexingStats #4195

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

Merged
merged 2 commits into from
Apr 3, 2025

Conversation

PeteGillinElastic
Copy link
Member

This adds the fields added in
elastic/elasticsearch#125521 and elastic/elasticsearch#124652 to the spec.

@PeteGillinElastic PeteGillinElastic added the skip-backport This pull request should not be backported label Apr 3, 2025
@PeteGillinElastic PeteGillinElastic changed the title Add new write load metrics to IndexingStats ES-10037 Add new write load metrics to IndexingStats Apr 3, 2025
Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Pete!

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

Actually, I believe you need to make generate the output changes also

@dakrone
Copy link
Member

dakrone commented Apr 3, 2025

@pquentin would it be possible to have something like make generate && git add … as part of a git precommit hook?

@PeteGillinElastic
Copy link
Member Author

@pquentin would it be possible to have something like make generate && git add … as part of a git precommit hook?

I believe that there's a bot which periodically comes along and commits any needed output changes. But it's not part of the same PR. (I've done make contrib and pushed it anyway. Which I think does slightly more than make generate does.)

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM still :)

@PeteGillinElastic PeteGillinElastic merged commit 47f94c3 into main Apr 3, 2025
8 checks passed
@PeteGillinElastic PeteGillinElastic deleted the new-write-load-metrics-in-indexing-stats branch April 3, 2025 16:44
@pquentin
Copy link
Member

pquentin commented Apr 4, 2025

@dakrone As @PeteGillinElastic mentioned, it's OK not to run those commands, as CI will notice if there's an issue and commit the output/ files. A pre-commit hook is an interesting idea, are you suggesting that I add that script to the repository and explain in CONTRIBUTING.md how to install it?

@dakrone
Copy link
Member

dakrone commented Apr 8, 2025

Ah okay, I hadn't realized that CI would do the commit for it. As for the pre-commit, what you described is what I was suggesting, but I don't think it's necessary unless we have to have the make run prior to merging (and we don't, since CI does it.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-backport This pull request should not be backported specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants