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

WIP: Accept Influx Line protocol #3708

Closed
wants to merge 4 commits into from

Conversation

gouthamve
Copy link
Contributor

So far accepts Telegraf content quite nicely. Need to clean up and think about the semantics of the API.

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
@pstibrany
Copy link
Contributor

Does this belong to Cortex itself? Could it be a proxy in front of Cortex, that translates Influx API to distributor calls instead?

@gouthamve
Copy link
Contributor Author

I think it should be part of Cortex tbh. Today Cortex is a really good Prometheus LTS. I think there are 3 things missing for it be a really good for general TSDB usecases:

  • Easy way for users to push data. My PR now and the future PR for accepting JSON.
  • A query language suited to gauges. I think we will see improvements in PromQL (last_over_time, etc.) once Prometheus starts accepting Push. But beyond that I can see Flux being added to the OSS codebase itself.
  • Accepting data out of order. This is the pitfall of push. I am not sure how to fix this, but I can see us tackling this after Loki tackles it.

Now the question is, do we want to be a generic TSDB? I don't know, I want it to be.

I do recognize the amount of dependencies we are pulling in, I will try to see if I can reduce that, but unlikely.

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
@pracucci
Copy link
Contributor

Reporting here the feedback I gave privately to Goutham.

I've mixed feeling about this. On one side I see the potential benefit to our users but, on the other side, we're creating a precedent to support more protocols (in the write path) and eventually more query languages (in the read path) which risks to add further maintenance burden to the project.

In this PR we're implicitly taking a bold decision of making Cortex speaking multiple protocols out of the box. We don't know how many users will leverage on it and how many protocols we'll end up supporting. An alternative approach may be building a separate proxy to put in front of Cortex first (so, fully decoupled from Cortex), see how the community reacts to it and eventually embed it into Cortex later. On the contrary, the opposite would be very difficult: once we add something to Cortex it's virtually impossible to get rid of it (or takes ages).

@pstibrany
Copy link
Contributor

pstibrany commented Jan 20, 2021

An alternative approach may be building a separate proxy to put in front of Cortex first (so, fully decoupled from Cortex), see how the community reacts to it and eventually embed it into Cortex later.

I agree with this approach (and is also feedback I gave internally).

Goutham:

Now the question is, do we want to be a generic TSDB? I don't know, I want it to be.

I don't think Cortex should try to be a generic TSDB. There are already projects that do that better, and Cortex' strength is in making Prometheus horizontally-scalable (wherever that eventually leads). If there is low-hanging fruit to make some use-cases easier, I'm all for it, but generic TSDB is completely different beast altogether (starting with query language, underlying storage, architecture choices).

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
This doesn't break Prometheus but 204 is required for Influx though.
Telegraf expects JSON on 200, and if it fails to parse the body, it
considers the send failed.

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
@khaines
Copy link
Contributor

khaines commented Jan 20, 2021

An alternative approach may be building a separate proxy to put in front of Cortex first (so, fully decoupled from Cortex), see how the community reacts to it and eventually embed it into Cortex later.

I agree with this approach (and is also feedback I gave internally).

I also agree with this approach ( I gave feedback on slack ). Decoupling the efforts, at least in the short term, doesn't commit the core project to support something that may not have broad adoption.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants