Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
MSC2063: Add "server information" public API proposal #2063
base: old_master
Are you sure you want to change the base?
MSC2063: Add "server information" public API proposal #2063
Changes from all commits
253f289
4b91bff
3f605b8
3dc8101
c59cd3b
0b06f97
5296824
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering what usefulness the uptime actually has? If it is small, it could be "they updated recently == good" or "they're in a crash loop doing constant OOM == bad". If it is big it could be either "it's stable" or "it's not being maintained". I don't know what would be better though, just wondering whether it is good to include numbers that don't really have any particular purpose. This feels more something that should live in the
/metrics
endpoint for prometheus.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do not need it doesn't mean nobody else does. I could describe how it's useful but really, I believe that "it is useful for some people" should be enough to justify.
This is a public endpoint, in contrast to prometheus. The primary point is to provide data for automated public server lists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh the first thing I'd disable is uptime: there's no metric that can be pulled from it which helps users with anything. How does uptime apply to a worker or multi-process homeserver for instance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the users viewpoint a service is "UP" when it's working. :-)
(As a user I would not pick a server with a constant less than a few days uptime, since it's either keep restarting or crashing, but that's me.)
But maybe you're right that it should be externally measured [icmp/api calls] instead of relying on the server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is almost perfect; but it'd be probably better to further distinguish between statistics/status information (uptime and the number of users) and configuration/settings information (open registrations). I don't know if it's worth it to introduce a separate
/server_config
endpoint just for a single boolean value - probably overengineering here. It would be great to have other opinions on this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
Processing a json and ignoring unnecessary fields is probably painless enough that it doesn't justify creating any more complexity.
I guess someone shall come up with a real justification to separate them even this way though, or maybe it's good for "visibility" or "mental separation"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather use two separate endpoints. The reason is that open registrations may (I can imagine that in theory at least) be configurable using some administration API. What I mean to say is that generally configuration is, at least in theory, read/write depending on your access credentials, while statistics are produced by the servers and cannot be tweaked even in theory. But probably let's validate it in the Matrix room.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be pointed out that open_registrations is not something federation should care about. It should be reported by a client API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh I think it's fine to expose health/metadata/metrics at the federation level for this, mostly because there's a discovery function that server lists can use to query the data.