fix: standardize logging in trip_details_handler

[Deepak8858]

This PR replaces direct slog.Warn calls with api.Logger.Warn in internal/restapi/trip_details_handler.go for consistency with other handlers and to ensure structured logging configuration is respected.\n\nFixes #694

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

Deepak Singh seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[tejasva-vardhan]

Hey @Deepak8858, welcome to Maglev and great start! 🎉 I'm a fellow contributor here, and I wanted to drop a quick review to help you get this PR ready for the maintainers.

You correctly fixed the slog.Warn instances! Just a couple of quick tips to make sure it fully passes the acceptance criteria for Issue #694:

1. Audit other files: The acceptance criteria for Standardize logging across REST API handlers #694 mentions checking all *_handler.go files in the internal/restapi/ directory to see if any others need fixing, not just trip_details_handler.go.

2. The Grep check: Issue Standardize logging across REST API handlers #694 states that running grep -r "slog\." must return zero matches. Looking at the diff, since slog.String(...) is still being used, that grep test will actually still fail. In the issue's Context section, the maintainer mentioned the pattern to follow is api.Logger.Warn("message", "key", value). If you swap slog.String("trip_id", trip.ID) to just "trip_id", trip.ID, you can completely remove the log/slog import and pass the grep test!

3. CLA Check: Also, it looks like the CLA bot is failing. You usually just need to link your git commit email in your GitHub settings and sign the agreement.

Let me know if you need any help with this, happy to help you get it merged!

[aaronbrethorst]

Hey Deepak, nice instinct picking this up — logging consistency is one of those things that matters more than people think. You've correctly identified the two slog.Warn calls in trip_details_handler.go and swapped them to api.Logger.Warn. That's the right fix for those lines.

However, this PR doesn't yet satisfy the acceptance criteria in issue #694. Let me walk you through what's still needed.

Critical Issues

1. Incomplete scope — issue #694 requires auditing all handler files

The issue's acceptance criteria state:

• Audit all *_handler.go files in internal/restapi/ for direct slog usage
• grep -r "slog\." internal/restapi/*handler*.go returns zero matches

Running that grep today still returns 26 matches across three handler files. Here's what still needs fixing:

trip_details_handler.go (this PR's file) — the slog.Warn calls are fixed, but slog.String(...) is still used as structured attrs on lines 165-166 and 175-176. Per the issue context, the pattern to follow is api.Logger.Warn("message", "key", value) with plain key-value pairs instead of slog.String(). Once you do that, you can drop the "log/slog" import entirely.

report_problem_with_stop_handler.go — uses slog.Default() on line 16 and slog.String(...) throughout.

report_problem_with_trip_handler.go — same pattern: slog.Default() on line 16 and slog.String(...) throughout.

2. slog.String usage keeps the log/slog import alive

Even after your change, trip_details_handler.go still imports "log/slog" because slog.String() is used as a log attribute. The acceptance criteria require zero slog. matches in handler files. Replace:

api.Logger.Warn("BuildTripStatus failed",
    slog.String("trip_id", trip.ID),
    slog.String("error", statusErr.Error()))

with:

api.Logger.Warn("BuildTripStatus failed",
    "trip_id", trip.ID,
    "error", statusErr.Error())

This lets you remove the "log/slog" import from this file.

Important Notes

Out-of-scope files also have slog usage

trips_helper.go has ~10 direct slog.Warn() calls (lines 67, 125, 259, 276, 292, 517, 530, 548, 996, 1013), and rate_limit_middleware.go uses slog.Default(). These aren't *_handler* files, so they're technically outside the grep check in #694's acceptance criteria. I'd suggest noting them in the PR description as follow-up work but keeping this PR focused on the handler files.

CLA needs signing

The CLA bot is flagging your PR. You'll need to link your commit email to your GitHub account and sign the CLA before this can be merged.

Suggestions

1. Fellow contributor @tejasva-vardhan already left great feedback — their comment covers the same issues I've noted. Follow their guidance and you'll be in good shape.

Recommended Action: Request Changes

1. Convert slog.String("key", value) to plain "key", value pairs in trip_details_handler.go and remove the "log/slog" import
2. Apply the same fixes to report_problem_with_stop_handler.go and report_problem_with_trip_handler.go
3. Verify grep -r "slog\." internal/restapi/*handler*.go returns zero matches
4. Run make test and make lint
5. Sign the CLA