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

Consider removing default port support #681

Closed
fabxc opened this Issue May 7, 2015 · 7 comments

Comments

Projects
None yet
2 participants
@fabxc
Copy link
Member

fabxc commented May 7, 2015

Currently target hosts can be provided without a port name. Based on the scheme the default port is selected.

Prometheus is not a browser and usage of default ports is an exception rather than the rule. Removing this feature will give developers and users one thing less to think about.

Thoughts?

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented May 11, 2015

Not sure if this behavior should be specific to a browser - port 80 and 443 are standard well-known default ports for HTTP and HTTPS, regardless of client application (http://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?&page=2). So it might even be less surprising to keep it.

@fabxc

This comment has been minimized.

Copy link
Member Author

fabxc commented May 11, 2015

Of course it's not specific to browsers — but for Prometheus, scraping from the default ports is not the normal case and a regular SD usually provides a port.
I just feel weird about interpolating the identifier for a scrape target, which should usually be a very explicit thing.

But not feeling too strong about it in general.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented May 12, 2015

Yeah, I don't feel strongly about this - do whatever feels best :)

@fabxc

This comment has been minimized.

Copy link
Member Author

fabxc commented May 13, 2015

Ultimate compromise: we infer the port when we receive the target group and thus only create targets from full <host>:<port>s. (And remove a potential dead lock with that.)

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented May 13, 2015

Can be closed now, I guess?

@fabxc

This comment has been minimized.

Copy link
Member Author

fabxc commented May 13, 2015

Yes

@fabxc fabxc closed this May 13, 2015

@lock

This comment has been minimized.

Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.