search

ribbybibby / s3_exporter

Exports Prometheus metrics about S3 buckets and objects
Apache License 2.0
105 stars 42 forks source link

Add new variable S3_ENDPOINT_URL #1

Closed stroitelev closed 5 years ago

stroitelev commented 5 years ago

Hello, I had a task to deploy the S3 exporter and integrate with our local S3 service. I have added new variable for that. Thank you for your work and maybe my little update will be helpful for you too.

ribbybibby commented 5 years ago

Thanks for the PR @stroitelev!

Could you please implement this variable as a kingpin flag, rather than reading the environment variable with os.Getenv()? A kingpin flag can be provided as an environment variable as well as a commandline flag so it provides more flexibility and also keeps configuration unified to kingpin.

https://godoc.org/github.com/alecthomas/kingpin#ArgClause.Envar https://godoc.org/github.com/alecthomas/kingpin#Application.DefaultEnvars

stroitelev commented 5 years ago

Hello @ribbybibby!

Sorry for delay, I have fixed it, could you check please? And sorry for my English, feel free to rewrite my descriptions if something wrong.

P.S. I also have helm chart for the s3-exporter, if it useful I can add it too.

stroitelev commented 5 years ago

About the environment variable S3_EXPORTER_S3_ENDPOINT_URL, I think it is well.

ribbybibby commented 5 years ago

Hi @stroitelev, your fork doesn't seem to use any environment variables now. You'll need to make a few more changes to enable kingpin's default envvars (see the example from my previous comment).

Also, the naming of endpointURL is inconsistently cased:

# github.com/stroitelev/s3_exporter
./s3_exporter.go:166:10: undefined: endpointUrl
./s3_exporter.go:167:21: undefined: endpointUrl
ribbybibby commented 5 years ago

Closing. I found some time to implement this myself. I've released it as 0.3.0.