-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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 ORCA UNIMPLEMENTED error to subchannel logger #10625
Conversation
Logging to the static instance would result in application logs filling up if the Orca service is not available. We'd like to have the logging on the subchannelLogger, so we make it visible on demand.
Using contains over containsExactly seems more reasonable.
@@ -153,7 +153,7 @@ private static OrcaLoadReportRequest buildOrcaRequestFromConfig( | |||
} | |||
|
|||
private static void assertLog(List<String> logs, String expectedLog) { | |||
assertThat(logs).containsExactly(expectedLog); | |||
assertThat(logs).contains(expectedLog); |
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 might be a questionable change, but I think this is the better way to test if a log message is present. However, I can rework this if needed.
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.
LGTM
Thank you! |
Logging to the static instance would result in application logs filling up if the Orca service is not available. We'd like to have the logging on the subchannelLogger, so we make it visible on demand. Also succeed Orca logging test if log message present. Using contains over containsExactly seems more reasonable.
Logging to the static instance would result in application logs filling up if the Orca service is not available. We'd like to have the logging on the subchannelLogger, so we make it visible on demand. Also succeed Orca logging test if log message present. Using contains over containsExactly seems more reasonable. Co-authored-by: Yannick Epstein <yannick.epstein@gmail.com>
We started using Orca, but not every service in our service mesh serves an Orca endpoint.
This means that our client side logs filled up with severe logging messages about the Orca service being
UNIMPLEMENTED
per subchannel.This is because the error is the only one that is logged to a static Java logger instance, while the rest of the subchannel messages are logged to the
SubchannelLogger
.In order to not fill up the logs of our services, we'd like to also log the
UNIMPLEMENTED
error to theSubchannelLogger
.