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

feat(awsS3Exporter): add support for s3_force_path_style and disable_ssl parameters #29331

Conversation

davinkevin
Copy link
Contributor

Description:

In order to support alternative object-storage, these parameters (s3_force_path_style and disable_ssl) are useful and help to leverage those object storage systems, not compatible with domain style path, or just hosted without ssl (like a minio pod deployed in a k8s namespace).

Testing::

I've tested it in this project: https://gitlab.com/davinkevin.fr/experimentations/opentelemetry/spring-boot-otel-to-minio

It was deployed in a k3d cluster and used to store metrics generated gathered by the prometheus receiver.

Documentation:, minimal, just description of the two new parameters, usually well known by administrators or operators in charge to connect systems to their S3-Compatible storages.

@atoulme
Copy link
Contributor

atoulme commented Nov 17, 2023

Please run make chlog-new at the root of the repository and enter information. Please open an issue or place this PR number in the issues field.

@atoulme
Copy link
Contributor

atoulme commented Nov 17, 2023

I think this looks good as you're just passing info to the aws.Config struct - is there any way to refer to AWS config links for folks in the README?

@davinkevin davinkevin force-pushed the support-extrat-parameters-to-be-compatible-with-minio branch 3 times, most recently from ff3935f to 9e3d330 Compare November 17, 2023 20:19
@davinkevin
Copy link
Contributor Author

👋 @atoulme,

I've updated the PR accordingly, thank you for your guidance 😇.

I think this looks good as you're just passing info to the aws.Config struct - is there any way to refer to AWS config links for folks in the README?

I've made the change in the doc for the s3_force_path_style, but there is no doc about the disable_ssl parameter.

If you want I can add a dedicated section about 3rd party s3 implementation, could be useful to those wanting to configure it with minio for example.

Let me know, I can do that in this PR or in a subsequent one, I let you decide 😇

@davinkevin davinkevin force-pushed the support-extrat-parameters-to-be-compatible-with-minio branch from 9e3d330 to 750d3aa Compare November 17, 2023 20:23
@atoulme
Copy link
Contributor

atoulme commented Nov 17, 2023

Subsequent PR sounds good. I think this is good to land as is.

@TylerHelmuth TylerHelmuth added the ready to merge Code review completed; ready to merge by maintainers label Nov 17, 2023
…le_ssl` parameters

In order to support alternative object-storage, these parameters are useful and help to leverage those systems not compatible with domain style path, or just hosted without ssl (like just deployed in a k8s namespace).
@davinkevin davinkevin force-pushed the support-extrat-parameters-to-be-compatible-with-minio branch from 750d3aa to d6bf72d Compare November 17, 2023 21:08
@TylerHelmuth TylerHelmuth merged commit 44b7b78 into open-telemetry:main Nov 17, 2023
83 checks passed
@github-actions github-actions bot added this to the next release milestone Nov 17, 2023
RoryCrispin pushed a commit to ClickHouse/opentelemetry-collector-contrib that referenced this pull request Nov 24, 2023
…le_ssl` parameters (open-telemetry#29331)

**Description:** 

In order to support alternative object-storage, these parameters
(`s3_force_path_style` and `disable_ssl`) are useful and help to
leverage those object storage systems, not compatible with domain style
path, or just hosted without ssl (like a minio pod deployed in a k8s
namespace).

**Testing:**: 

I've tested it in this project:
https://gitlab.com/davinkevin.fr/experimentations/opentelemetry/spring-boot-otel-to-minio

It was deployed in a k3d cluster and used to store metrics generated
gathered by the prometheus receiver.

**Documentation:**, minimal, just description of the two new parameters,
usually well known by administrators or operators in charge to connect
systems to their S3-Compatible storages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter/awss3 ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants