Move remote write API to client_golang/exp #1711
Merged
Conversation
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
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
2 tasks
|
I would straight away try to avoid old client shenigans and rethink this from scratch. |
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
saswatamcode
commented
Jan 18, 2025
Comment on lines
+41
to
+42
| baseURL *url.URL | ||
| client *http.Client |
There was a problem hiding this comment.
Passing http.Client directly here instead of api.Client structs. Seems like the most flexible way, without doing custom configs.
| // SnappyDecompressorMiddleware returns a middleware that checks if the request body is snappy-encoded and decompresses it. | ||
| // If the request body is not snappy-encoded, it returns an error. | ||
| // Used by default in NewRemoteWriteHandler. | ||
| func SnappyDecompressorMiddleware(logger *slog.Logger) func(http.Handler) http.Handler { |
There was a problem hiding this comment.
Tried to convert this into a middleware with the ability for downstream users to override and add their own middlewares if needed.
Let me know what you think, or if this is too much
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
|
Can we create a go mod file? It has to be a separate Go module if it will be experimental. |
kakkoyun
reviewed
Jan 20, 2025
There was a problem hiding this comment.
Looking good.
We need documentation:
- We need a README. And we need to make the intention crystal clear.
- Also, we have to be clear, we don't give any guarantees that the experimental APIs won't introduce breaking changes, or they will eventually be promoted to stable.
- How do we release? This suppose to have different release cycles and maybe tags.
Additionally, we need experiment owners. So that we can ping them on the issues. |
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
saswatamcode
commented
Jan 20, 2025
go.work
Outdated
There was a problem hiding this comment.
Added a workspace file as well
Yup, probably some example usage helps
exp package go doc has a line on this, but yes probably something to mention in dedicated README. Might be good to do this in next PR to prwclient-em branch? |
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
bwplotka
reviewed
Jan 31, 2025
| go 1.21 | ||
|
|
||
| use ( | ||
| . |
bwplotka
approved these changes
Jan 31, 2025
bwplotka
added a commit
that referenced
this pull request
Feb 25, 2025
…t and handler. (#1658) * api: Add remote API with write client; add remote handler. Signed-off-by: bwplotka <bwplotka@gmail.com> * Make Write message type more flexble, address some feedback (#1710) * Address remaining feedback Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Make Write message type more flexible Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> --------- Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Move remote write API to client_golang/exp (#1711) * Move remote write API to client_golang/exp Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Don't use api.Client structs, add options for middleware Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Fix reqBuf usage Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Fix url path Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Add separate mod file (and workspace file) Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Hook exp tests fmt; Test handler error case; Configure backoff Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> --------- Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * exp: Add README, address feedback, use sync.Pool (#1747) * Implement suggestion for Store interface and contentType Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Add README Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Use sync.Pool Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Implement review suggestions Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Release bufs right after compressPayload Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> --------- Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Bump exp to go 1.22 Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> --------- Signed-off-by: bwplotka <bwplotka@gmail.com> Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> Co-authored-by: Saswata Mukherjee <saswataminsta@yahoo.com>
ying-jeanne
pushed a commit
to ying-jeanne/client_golang
that referenced
this pull request
Mar 28, 2025
…t and handler. (prometheus#1658) * api: Add remote API with write client; add remote handler. Signed-off-by: bwplotka <bwplotka@gmail.com> * Make Write message type more flexble, address some feedback (prometheus#1710) * Address remaining feedback Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Make Write message type more flexible Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> --------- Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Move remote write API to client_golang/exp (prometheus#1711) * Move remote write API to client_golang/exp Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Don't use api.Client structs, add options for middleware Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Fix reqBuf usage Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Fix url path Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Add separate mod file (and workspace file) Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Hook exp tests fmt; Test handler error case; Configure backoff Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> --------- Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * exp: Add README, address feedback, use sync.Pool (prometheus#1747) * Implement suggestion for Store interface and contentType Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Add README Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Use sync.Pool Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Implement review suggestions Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Release bufs right after compressPayload Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> --------- Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> * Bump exp to go 1.22 Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> --------- Signed-off-by: bwplotka <bwplotka@gmail.com> Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com> Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> Co-authored-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Moving to exp.
Do we want to create new client in this remote write API addition as well? Or do future iteration on it? :)
I might add the decompression middleware stuff here as well
cc: @bwplotka