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

*: Implement info server for missing components #4886

Merged

Conversation

matej-g
Copy link
Collaborator

@matej-g matej-g commented Nov 21, 2021

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

This PR is the continuation of the series of PRs to resolve the issue #2600 implementing the endpoints proposal.

The changes include:

  • Implementing the info server for remaining components - querier, receiver and ruler. This includes also adjustments to store implementation to make it possible to obtain necessary store information (time range, label set)
  • Some improvements to info package and info server options (mainly simplifies providing default non-empty method implementations)
  • Simplifies some parts of endpoint package (removes unnecessary interface and min/max timestamps)
  • Re-enables getting endpoint metadata via store info method, since now all components in latest version implement the info server.

Verification

Tests have been adjusted and passed locally. Manual test has been executed as well.

- Adds time range and label set methods to store to be able to populate info API responses

Signed-off-by: Matej Gera <[email protected]>
…info

- Makes some changes to info server options for better readability
- Makes passing func optional - if no function passed, sets default empty response
- Adjust existing components to fit adjusted info server options

Signed-off-by: Matej Gera <[email protected]>
- Rely on info API response since all components in latest version implement it now
- Simplify, remove an unnecessary interface and min/max time const

Signed-off-by: Matej Gera <[email protected]>
@onprem onprem self-requested a review November 22, 2021 05:47
onprem
onprem previously approved these changes Nov 22, 2021
Copy link
Member

@onprem onprem left a comment

Choose a reason for hiding this comment

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

Nice work @matej-g! This is looking good to go, just some small non-blocking suggestions.

Makefile Show resolved Hide resolved
pkg/info/info.go Outdated Show resolved Hide resolved
pkg/info/info.go Outdated Show resolved Hide resolved
- Simplify the functional option - remove interface
- Better documentation

Signed-off-by: Matej Gera <[email protected]>
Copy link
Member

@onprem onprem 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 for this!

Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

💪 awesome work!

@GiedriusS
Copy link
Member

Ran tests locally and seems like everything works as expected

@GiedriusS GiedriusS merged commit 3d1b254 into thanos-io:main Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants