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

[NEW] Add prometheus port config #11115

Merged
merged 13 commits into from
Jun 15, 2018

Conversation

thaiphv
Copy link
Contributor

@thaiphv thaiphv commented Jun 13, 2018

[NEW] Allow the Prometheus port to be configurable

Closes #11114

This also picks a default port number that doesn't conflict with existing allocations from https://github.com/prometheus/prometheus/wiki/Default-port-allocations. Per the recommendation, I use the 9451 port, which is available after the one allocated to Habitat Exporter

@CLAassistant
Copy link

CLAassistant commented Jun 13, 2018

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
3 out of 4 committers have signed the CLA.

✅ stuartpb
✅ engelgabriel
✅ thaiphv
❌ brylie
You have signed the CLA already but the status is still pending? Let us recheck it.

@geekgonecrazy
Copy link
Contributor

My only problem is this now becomes a breaking change. Because it was released with 9100 already, so if we change the default then when people upgrade it's broken

@geekgonecrazy geekgonecrazy requested a review from rodrigok June 13, 2018 05:43
@thaiphv
Copy link
Contributor Author

thaiphv commented Jun 13, 2018

My only problem is this now becomes a breaking change. Because it was released with 9100 already, so if we change the default then when people upgrade it's broken

I'm OK with not setting a default value of 9100 as we haven't actually reserved any port for the service.

@thaiphv
Copy link
Contributor Author

thaiphv commented Jun 14, 2018

I've changed the default value of Prometheus_Port to 9100 so that it doesn't introduce a backward incompatible change.

@thaiphv
Copy link
Contributor Author

thaiphv commented Jun 14, 2018

Can someone please take a look at this change?

@thaiphv thaiphv changed the title Add prometheus port config [NEW] Add prometheus port config Jun 14, 2018
@engelgabriel engelgabriel merged commit 873ae7d into RocketChat:develop Jun 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants