show response result in message traces #6359
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
grondo
approved these changes
Oct 10, 2024
|
Yeah could probably add some tests. |
|
Added a little bit more testing (though it didn't do much for the numbers). Anyway, setting MWP. |
Problem: the numerical constants used in overlay control messages are defined locally to overlay.c but it is useful to decode them in a message trace. Move to overlay.h.
Problem: the broker contains duplicate code for tracing module and overlay messages. Factor out the duplicate code to trace.[ch]. Eliminate the check at each call site for empty 'trace_requests' msglist and put the check in the trace function. Add more defensive checks in the trace function. The response payload now contains both 'name' and 'rank' fields, with "name":null in the overlay response and "rank":-1 in the module response. Both clients already allow extra fields so the clients need not change.
Problem: if there are multiple message tracers and one sets the 'full' flag, all tracers get the message payload. Only send the message payload if requested.
Problem: flux module trace and flux overlay trace do not show whether a response indicates success or failure, but this would be useful. Include the response errnum and error string (if available) in the trace response.
Problem: strerrorname_np() is unavailable on non-glibc systems. Add an implementation from gnulib to libmissing. the license is LGPL-2.1+.
Problem: flux module trace doesn't show whether response messages are a success or failure. Add the errnum value from the message, converted to its short error name, to the end of the message line. If there is a human readable error message in the payload, show it when --full is specified. Show error responses in bright red rather than cyan.
Problem: flux overlay trace doesn't show whether response messages are a success or failure. Add the errnum value from the message, converted to its short error name, to the end of the message line. If there is a human readable error message in the payload, show it when --full is specified. Show error responses in bright red rather than cyan.
Problem: when flux-overlay trace encounters an error decoding the trace response, it prints errors twice. Use flux_msg_exit() not flux_err_exit().
Problem: there is no test coverage for flux module trace and flux overlay trace's special handling of response messages. Add a test for a failing RPC with human-readable error.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6359 +/- ##
==========================================
- Coverage 83.31% 83.28% -0.03%
==========================================
Files 523 524 +1
Lines 86199 86224 +25
==========================================
- Hits 71814 71813 -1
- Misses 14385 14411 +26
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem:
flux module traceandflux overlay tracedon't show theerrnumvalue for response messages, but this would be helpful when interpreting message traces.Append "success" or the error name associated with the
errnumvalue in the one line trace output for responses.When
--fullis specified, include human readable error message (if any) from the payload.Change the color of failure response messages to bright red so they stand out.