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

Need a way to stop tempo from using dual stack urls for s3 backend #3720

Closed
rohan-changejar opened this issue May 28, 2024 · 13 comments · Fixed by #3721
Closed

Need a way to stop tempo from using dual stack urls for s3 backend #3720

rohan-changejar opened this issue May 28, 2024 · 13 comments · Fixed by #3721

Comments

@rohan-changejar
Copy link

rohan-changejar commented May 28, 2024

Tempo compactor using s3 dualstack urls to interact with s3 backend storage, need a way to turn this behaviour off

  • Tempo compactor using s3 dual stack urls which are not supported by vpc private link services, provided by aws. With this feature not available, the cost we are bearing due to transfer through network egress methods such as nat gateway are very high. We need a provision to reset this behaviour to the default manner of using the default s3 url which supports vpc endpoints

To Reproduce
Steps to reproduce the behaviour:

  1. Start Tempo -
    tempo-distributed-1.9.1 2.4.1
  2. Perform Operations (Read/Write/Others)

Expected behaviour

  • There should be an option to control this behaviour of s3 upload and ensure that we can choose between dualstack endpoints and native endpoints

Environment:

  • Infrastructure: Kubernetes
  • Deployment tool: helm
@tejasraj-jar
Copy link

Facing the same issue

1 similar comment
@Naresh-Srinivas
Copy link

Facing the same issue

@shubhamivane
Copy link

Same here :(.

@mapno
Copy link
Member

mapno commented May 28, 2024

Hi! We use minio-go as client in Tempo for S3. The ability to enable/disable dualstack endpoints was recently added and seems to already be available in the latest release.

Support for this in Tempo would require updating the client and adding a new config option for S3 backends to control it. Would you be up for this change?

@rohan-changejar
Copy link
Author

Yes, certainly up for the change, this has significant cost impact for us. Can you please guide us through the process

@rohan-changejar
Copy link
Author

image adding here for information

@mapno
Copy link
Member

mapno commented May 28, 2024

You'll need to update the library minio/minio-go to the v7.0.70 release (you can run make vendor-check to validate that the library is correctly updated) and use the new method SetS3EnableDualstack to enable/disable this option in S3 clients. The code for S3 clients lives here.

There is also a contributing guide if you have more general questions.

@sid-jar
Copy link
Contributor

sid-jar commented May 28, 2024

Screenshot 2024-05-28 at 3 04 02 PM @mapno, firstly thanks for the suggestion, but a further question being.. we have disabled minio explicitly to be used with tempo. So does tempo use minio internally to compact and send blocks over to s3 ??

@mapno
Copy link
Member

mapno commented May 28, 2024

No, we use minio-go, which is a "client SDK for S3 compatible object storage". It's just a library to connect to S3-compatible backends.

@sid-jar sid-jar mentioned this issue May 28, 2024
3 tasks
@Shreyank031
Copy link

Shreyank031 commented Jun 3, 2024

@mapno The s3DualstackEnabled bool is not added/updated on the vendor packages (api.go). But on the minio source code, we can see s3DualstackEnabled bool in struct. Could this create the conflict when we try to enable it by force ??

@mapno
Copy link
Member

mapno commented Jun 3, 2024

I'm not sure I understand what you mean.

S3 dualstack endpoints are configured via the method (function) SetS3EnableDualstack

@Shreyank031
Copy link

Shreyank031 commented Jun 6, 2024

Hi @mapno , Had there been any updates on the functionality implementation part. We're also facing a similar issue and It'd greatly help our use case to not use s3 on a dualstack mode. Let us know if we can help in there somehow, but would appreciate a faster expedition. Though Tempo is fairly new to me, I've been practicing Go for sometime now. Let me know if I can contribute here. If its fine, I can create a fresh PR and you can review and point me in the right direction. Thanks

@mapno
Copy link
Member

mapno commented Jun 6, 2024

Hi. Yes, you're welcomed to open a new PR or continue this one #3721. I believe I've explained what needs to be done in the multiple comments in the PR (see #3721 (comment)). Thanks!

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

Successfully merging a pull request may close this issue.

7 participants