-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(client): add usage report config #2586
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
Conversation
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
jyyi1
left a comment
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.
Thanks for the implementation. Logic is clear, added a few comments.
Co-authored-by: J. Yi <[email protected]>
Co-authored-by: J. Yi <[email protected]>
fortuna
left a comment
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.
Thanks for the quick review. I'll try to address them tomorrow morning.
jyyi1
left a comment
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.
Thanks, LGTM now. It would be good to go after fixing the race condition caught by CI.
fortuna
left a comment
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.
Race condition fixed with the Go 1.25 upgrade, which also brings performance and security improvements from Go 1.23 and 1.24.
|
FYI, I had to fix an issue caused by the upgrade to 1.25 and Go Mobile: golang/go#71827 |
|
What is the use case scenario for this reporter that sends a POST request to a server periodically? |
|
@emohandesi You guys requested it (every 24h). We can remove it though. |
OK, I see. I thought there was another use case for it. I was trying to figure out if we could somehow take advantage of it. |
|
The new update introduced usage reporting via the HTTP reporter, which sends a POST request to the configured URL at the start of the session and then at every interval. However, I noticed that it only sends the request if the connection is successful. With this method, is it possible to also receive error messages or connection failure reports through the same reporting mechanism? |
No, sending the connection error is not possible in this feature. #2036 and #2484 are implementing the error reporting feature. |
|
Hello, @fortuna, could you give me some insight into how this feature works and what the use case for it is? I have been looking for this feature's documentation on Google Developer, but it seems like this feature has not been added to the documentation yet. Edit: Doc found on https://github.com/Jigsaw-Code/outline-apps/blob/master/client/config.md |
This PR adds support for usage reporting. The config format looks like this:
The
httpreporter will send a POST request to the given URL at the beginning of the session, and then everyinterval./cc @emohandesi @62w71st