add detail about interaction with zero injection - #87
Conversation
Signed-off-by: Owen Williams <owen.williams@grafana.com>
bwplotka
left a comment
There was a problem hiding this comment.
Thanks! We can also clarify perhaps what can we improve e.g. adding "stZeroInjection" to scrape config in future and removing FF flag (if you agee).
Signed-off-by: Owen Williams <owen.williams@grafana.com>
done. punted :) |
|
|
||
| #### Interaction with `created-timestamps-zero-injection` | ||
|
|
||
| The existing zero-injection feature is useful and has been around long enough that some large users are relying on it. The new ST support will be in addition to or an alternative to zero-injection. In order to better differentiate zero-injection from st-per-sample, we propose to rename `created-timestamps-zero-injection` to `start-time-injection-at-scrape`. |
There was a problem hiding this comment.
I don't think we want to rename feature flag.
There was a problem hiding this comment.
ok, someone in the meeting mentioned renaming it so that's why I wrote it down
|
|
||
| The existing zero-injection feature is useful and has been around long enough that some large users are relying on it. The new ST support will be in addition to or an alternative to zero-injection. In order to better differentiate zero-injection from st-per-sample, we propose to rename `created-timestamps-zero-injection` to `start-time-injection-at-scrape`. | ||
|
|
||
| We also propose that the st setting be be tri-state on a per-series basis. Start Time per Sample support can be either `off`, `scrape`, or `per-sample`. Large installations may have a mix of implementations and there is no compatibility concern with having series have differing settings. |
There was a problem hiding this comment.
Are we sure we want per series? Can we configure it per series?
I'd say per scrape job is perhaps enough.
either
off,scrape, orper-sample
Do we mean scrape job is planned to be either off, stZeroInjection, stPerSample?
There was a problem hiding this comment.
I was hoping to add just enough text to indicate the intent, not the implementation, so we could merge the proposal
| We also propose that the st setting be be tri-state on a per-series basis. Start Time per Sample support can be either `off`, `scrape`, or `per-sample`. Large installations may have a mix of implementations and there is no compatibility concern with having series have differing settings. | ||
|
|
||
| In the case where a series has both scrape-injected zeroes and st-per-sample, the scrape-injected zeros take precedence. |
There was a problem hiding this comment.
Isn't above already blocking this (tri state)?
Also you can't detect that after a fact and this statement assumes we will?
There was a problem hiding this comment.
This refers to when tsdb blocks being queried have both, for some reason. Ideally we shouldn't end up with data like that.
|
|
||
| In the case where a series has both scrape-injected zeroes and st-per-sample, the scrape-injected zeros take precedence. | ||
|
|
||
| We also intend to graduate zero-injection from an experimental feature, which could involve replacing the existing flag with, for example, a `stZeroInjection` term to the scrape config. The exact implementation is out of scope of this proposal. |
There was a problem hiding this comment.
So why we explained before exactly how we planned it? (:
There was a problem hiding this comment.
People asked for per-series configuration, so that's what I wrote down, but I didn't want to specify exactly where that configuration would go. You mention scrape job, other people seemed to agree with "per series"... that's why I didn't want to continue to slow down this PR with that detail and leave it for implementation
as per the meeting notes