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

Log more backend endpoint info #287

Merged
merged 2 commits into from
Aug 17, 2021
Merged

Log more backend endpoint info #287

merged 2 commits into from
Aug 17, 2021

Conversation

46bit
Copy link
Contributor

@46bit 46bit commented Jul 13, 2021

  • A short explanation of the proposed change: log a little more information when backend endpoints fail

  • An explanation of the use cases your change solves: we are experiencing a small number of backend failures during platform updates. We would like a little more information in the logs, as provided by this PR

  • Instructions to functionally test the behavior change using operator interfaces (BOSH manifest, logs, curl, and metrics): can trigger this log by stopping a traffic-receiving VM with bosh stop --skip-drain diego-cell/N

  • Expected result after the change: extra context in the logs, like can be seen here and in the PR:{"log_level":3,"timestamp":"2021-07-13T15:01:30.452379948Z","message":"backend-endpoint-failed","source":"vcap.gorouter","data":{"retriable":true,"num-endpoints":1, [...]}}

  • Current result before the change: the retriable and num-endpoints fields aren't present, which makes it a little harder to follow what behaviour Gorouter will do next

  • I have viewed signed and have submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using scripts/run-unit-tests-in-docker from routing-release.

  • I have deployed this in a CF environment and checked the new log fields print as expected

  • (Optional) I have run Routing Acceptance Tests and Routing Smoke Tests on bosh lite

  • (Optional) I have run CF Acceptance Tests on bosh lite

Michael Mokrysz added 2 commits July 8, 2021 11:25
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.

The labels on this github issue will be updated when the story is started.

@46bit
Copy link
Contributor Author

46bit commented Jul 13, 2021

One more thing we'd like to do is make it easier for clients to understand why a request failed without us having to examine our internal logs. We have a draft commit for that. Any thoughts if you're comfortable leaking each of these situations?

@ameowlia ameowlia merged commit b908500 into cloudfoundry:main Aug 17, 2021
@ameowlia
Copy link
Member

ameowlia commented Aug 17, 2021

Thank you @46bit for this PR.

In general, I am very supportive of making cf_router_error header more specific and helpful to end users. I worry a little bit about leaking security concerns, but I can't think of any specific problems right now.

@46bit 46bit deleted the log-more-backend-endpoint-info branch August 17, 2021 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants