feat(zetaclient): add /systemtime telemetry endpoint#3235
Conversation
gartnera
requested review from
brewmaster012,
kingpinXD,
lumtis,
skosito,
swift1337 and
ws4charlie
as code owners
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces a new HTTP handler, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3235 +/- ##
===========================================
- Coverage 61.81% 61.81% -0.01%
===========================================
Files 431 431
Lines 30759 30763 +4
===========================================
Hits 19015 19015
- Misses 10886 10890 +4
Partials 858 858
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
zetaclient/metrics/telemetry.go (2)
337-341: Add unit tests for the new endpointThe new endpoint lacks test coverage. Consider adding the following test cases:
- Verify the response format (RFC3339)
- Verify the content type header
- Verify the time is in UTC
Would you like me to help generate the unit test implementation?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 338-340: zetaclient/metrics/telemetry.go#L338-L340
Added lines #L338 - L340 were not covered by tests🪛 GitHub Check: lint
[failure] 340-340:
unnecessary conversion (unconvert)
337-341: Consider adding response cachingSince system time queries might be frequent during debugging, consider adding caching headers to help clients make efficient use of the endpoint.
func systemTimeHandler(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "text/plain") + w.Header().Set("Cache-Control", "public, max-age=1") nowString := time.Now().UTC().Format(time.RFC3339) _, err := fmt.Fprint(w, nowString) if err != nil { log.Error().Err(err).Msg("Failed to write response") } }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 338-340: zetaclient/metrics/telemetry.go#L338-L340
Added lines #L338 - L340 were not covered by tests🪛 GitHub Check: lint
[failure] 340-340:
unnecessary conversion (unconvert)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
zetaclient/metrics/telemetry.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
zetaclient/metrics/telemetry.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
🪛 GitHub Check: codecov/patch
zetaclient/metrics/telemetry.go
[warning] 194-194: zetaclient/metrics/telemetry.go#L194
Added line #L194 was not covered by tests
[warning] 338-340: zetaclient/metrics/telemetry.go#L338-L340
Added lines #L338 - L340 were not covered by tests
🪛 GitHub Check: lint
zetaclient/metrics/telemetry.go
[failure] 340-340:
unnecessary conversion (unconvert)
lumtis
left a comment
lumtis
left a comment
There was a problem hiding this comment.
LGTM outside of lint issue
gartnera
force-pushed
the
zetaclient-system-time
branch
from
56d2f08 to
75e050b
Compare
3 participants
This endpoint will be used to debug issued related to bad system time values.
Related to #3227
Summary by CodeRabbit
/systemtimethat returns the current system time in UTC format./systemtimeendpoint functionality.