Add tests for /api/v1/target/metadata#6409
Merged
brian-brazil merged 3 commits intoprometheus:masterfrom Dec 5, 2019
Merged
Conversation
…etriever Previously, the struct `testTargetRetriever` had hardcoded active and dropped targets. This made it difficult to change the target information depending on the test case. This change introduces a way to define them as arguments and pass it to a constructor for building. It lays a foundation for dynamically defining targets with various set of arguments to test different scenarios. Signed-off-by: gotjosh <josue@grafana.com>
To test the implementation of our metric metadata API, we need to represent various states of metadata in the scrape metadata store. That is currently not possible as the interface and method to set the store are private. This changes the interface, list and get methods, and the SetMetadaStore function to be public. Incidentally, the scrapeCache implementation needs to be renamed to match the new signature. Signed-off-by: gotjosh <josue@grafana.com>
This commit introduces several test cases for the current /targets/metadata API endpoint. To achieve so, we use a mock of the metadataStore and inject it to the targets under test. Currently, three success cases are covered: with a metric name, with a target matcher, and with both. As for the failure scenario, the one where we couldn't match against a particular metric is covered. Signed-off-by: gotjosh <josue@grafana.com>
1dc484c to
428089f
Compare
Contributor
We're generally fine with making things public, even if we don't need that visibility within Prometheus. |
gotjosh
commented
Dec 5, 2019
| ) | ||
|
|
||
| func (t testTargetRetriever) TargetsActive() map[string][]*scrape.Target { | ||
| testTarget := scrape.NewTarget( |
Member
Author
There was a problem hiding this comment.
While this change is technically not needed right now. It will in my subsequent pull request as I need to define various targets at different tests cases dynamically.
Would rather get some feedback on it now and avoid a bigger diff later on.
Contributor
|
Thanks! |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As a foundation for #6395, let's add test to the existing metadata API.
The only thing that could be mildly controversial is that I've had to make the interface
MetricMetadataStorepublic alongside with its methods.Without it, we have no way (to the best of my knowledge) to represent data in the metadata cache of a specific target. The current design of the
scrapepackage tells us that the cache should be assigned to a scrape pool instead of individual targets.I tried hardly to circumvent this challenge and avoid changing the interface but that proved futile. All in all, this is the minimal diff I can put together to make this work - there are other alternatives but I get the feeling these are more involved and would require an overhaul of the current testing structure e.g. Using the
scrape/managerto set the cache for the pool.Best viewed commit-by-commit.
TODO