sidecar: Allow customizing the shipper metadata file name #6886
Conversation
This file contains 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
sinkingpoint
force-pushed
the
sinkingpoint/custom-shipper-paths
branch
3 times, most recently
from
d2f8df5
to
d93eccf
Compare
fpetkovski
approved these changes
Nov 14, 2023
Currently, the shipper metadata file is always called `thanos.shipper.json` in the Prometheus data directory. This precludes running multiple sidecars that upload to different object stores, as they will overwrite each other's metadata file. This commit allows the metadata file name to be customized via a flag. The default is unchanged, but it can be overridden with the `--shipper.meta-file-name` flag. As part of this, we update the signatures of `WriteMetaFile` and `ReadMetaFile` to take the full path of the metadata file, rather than just the directory, and updates the tests that go along with this. Signed-off-by: sinkingpoint <[email protected]>
sinkingpoint
force-pushed
the
sinkingpoint/custom-shipper-paths
branch
from
d93eccf
to
c1305d3
Compare
yeya24
approved these changes
Nov 16, 2023
|
Seems like the doc test is unrelated? |
|
Yeah seems to be either flaky or broken for some other reason. |
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.
Currently, the shipper metadata file is always called
thanos.shipper.jsonin the Prometheus data directory. This precludes running multiple sidecars that upload to different object stores, as they will overwrite each other's metadata file.This commit allows the metadata file name to be customized via a flag. The default is unchanged, but it can be overridden with the
--shipper.meta-file-nameflag.As part of this, we update the signatures of
WriteMetaFileandReadMetaFileto take the full path of the metadata file, rather than just the directory, and updates the tests that go along with this.The previous guidance on doing this was to run a single sidecar to upload to one object storage, and then sync that across to a second. In our case our sidecars run outside the clouds that store our data, so doing this becomes cost prohibitive as we have to pay egress fees from one cloud to the other. Uploading to both from our Prometheus is much cheaper as ingress is orders or magnitude less expensive.
A few questions on the implementation here:
Read/Writemethods pretty closely to the shipper itself. Does it make sense to move them under it?Changes
shipper.meta-file-nameflag, allowing customizing the shipper meta file path, and allowing sidecars to co-existVerification
Mostly submitting this to get comments on the approach for now. We run a variant of this at Cloudflare, but less formal. I'm testing this patch precisely next week. For now, I'll a test one to test the customization