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

Allow swapping out PromQL engine in web API #5603

Closed
wants to merge 1 commit into from

Conversation

juliusv
Copy link
Member

@juliusv juliusv commented May 28, 2019

Currently we allow swapping out the Queryable that the *promql.Engine uses, but
if one wants to implement PromQL evaluation by an entirely different mechanism
than the *promql.Engine, this upper layer also needs to be swappable. The
concrete motivation for this is for adding a Prometheus API endpoint to
InfluxDB which handles PromQL queries by transpiling them to Flux and then
executing the resulting Flux query instead.

Signed-off-by: Julius Volz julius.volz@gmail.com

Currently we allow swapping out the Queryable that the *promql.Engine uses, but
if one wants to implement PromQL evaluation by an entirely different mechanism
than the *promql.Engine, this upper layer also needs to be swappable. The
concrete motivation for this is for adding a Prometheus API endpoint to
InfluxDB which handles PromQL queries by transpiling them to Flux and then
executing the resulting Flux query instead.

Signed-off-by: Julius Volz <julius.volz@gmail.com>
@roidelapluie
Copy link
Member

Since we import promql anyway, should we move the interface to the promql package and use that in:

cmd/promtool/
web/api/v1/
rules/

?

@juliusv
Copy link
Member Author

juliusv commented Jan 21, 2020

@roidelapluie So a common pattern in Go is to define an interface at a usage site (rather than at the site of an implementation) if you need a narrower interface than the actual implementation type, in the sense that the interface here only contains the methods required by the API. The concrete *promql.Engine has more methods than just these two (actually just one more at the moment, but could be more).

But if that narrower interface is required in multiple places, it could make sense to move it to a shared place, yeah. If we moved it to the promql package, then we should use interface composition there, with one narrow interface just defining the two methods required here (and in promtool, etc.), and another interface embedding that, plus adding the one more other method.

But I'm not sure that's needed at the moment. Initially I had filed this PR for a specific reason related to the PromQL->Flux transpiler, I believe. But I think the reason became obsolete. I'll close it for now.

@juliusv juliusv closed this Jan 21, 2020
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.

None yet

2 participants