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

Improve debug logging #3361

Merged
merged 1 commit into from
Dec 14, 2023
Merged

Improve debug logging #3361

merged 1 commit into from
Dec 14, 2023

Conversation

m-horky
Copy link
Contributor

@m-horky m-horky commented Dec 11, 2023

This change clearly labels each section that's printed to make it easier to identify the interesting parts.

This also adds SUBMAN_DEBUG_PRINT_TRACEBACKS flag to print the traceback directly to stdout.


Example:

image
(the white horizontal line is just result of putting two screenshots together, the traceback was too long)

@cnsnyder cnsnyder requested review from a team and wottop and removed request for a team December 11, 2023 13:27
Copy link

github-actions bot commented Dec 11, 2023

Coverage

Coverage (computed on Fedora latest) •
FileStmtsMissCoverMissing
cloud_what
   _base_provider.py2064876%42, 144, 151, 169, 187, 199, 207–208, 219–221, 228–229, 250, 287–289, 304–306, 312–313, 318, 353, 362–363, 365–367, 369–371, 373, 384–385, 387–388, 390–391, 393, 412, 417, 450, 459, 478–479, 483, 499
rhsm
   connection.py101646154%48–49, 53, 55–56, 81, 101–102, 150, 284, 315, 381–386, 390–399, 460, 462, 564, 567, 574–580, 585, 641, 676–680, 682, 695, 722, 725–726, 728–729, 731, 742–746, 750, 754, 756–757, 784, 787, 791–792, 797, 800–801, 816, 820, 822–823, 850–851, 853, 856, 861–862, 865–866, 868, 870–874, 876–877, 880–887, 889–899, 901, 903–904, 915–917, 919–921, 923–925, 927–929, 931, 934–940, 942–943, 945–946, 948, 959–961, 963–964, 966–968, 970, 982–985, 990, 1054, 1056–1061, 1063, 1068–1072, 1078–1081, 1083–1088, 1092–1097, 1104, 1141, 1143, 1148, 1159, 1168–1171, 1175, 1177–1179, 1183–1184, 1186–1193, 1195, 1197, 1200–1207, 1210–1211, 1216, 1218, 1268, 1285–1288, 1312, 1334, 1364, 1369, 1372, 1375–1376, 1381, 1384, 1389, 1392, 1435–1439, 1446–1447, 1449, 1457–1458, 1460, 1477, 1490–1492, 1495, 1508, 1515, 1519, 1547–1549, 1554–1555, 1557–1558, 1560–1561, 1563–1577, 1579–1581, 1583–1594, 1596, 1613–1615, 1617–1619, 1621–1623, 1628, 1633–1635, 1640, 1667, 1699–1727, 1732–1733, 1735–1737, 1740–1741, 1744–1745, 1748–1749, 1768–1769, 1778–1779, 1789–1790, 1797–1798, 1804–1807, 1813–1816, 1822–1823, 1829–1830, 1850–1851, 1860–1864, 1872–1873, 1899–1902, 1927–1928, 1937–1938, 1946–1947, 1968, 1970–1972, 1974, 1976, 1979, 1981–1994, 1996–1997, 2006–2008, 2020–2021, 2030–2031, 2033, 2035–2037, 2044–2046, 2055–2057, 2065–2066, 2077, 2079–2080, 2082, 2084–2087, 2089–2091, 2094, 2096, 2103–2104, 2111–2112, 2122–2123, 2133–2136, 2146–2152, 2159–2162
   utils.py2995182%48, 230, 324, 344, 432, 434, 438, 441–443, 446–448, 451, 454–456, 532, 538–539, 544, 553–557, 559–560, 566–568, 575–579, 582–584, 586–587, 589–590, 593–595, 599–603
TOTAL18185459774% 

Tests Skipped Failures Errors Time
2640 14 💤 0 ❌ 0 🔥 48.241s ⏱️

@m-horky m-horky marked this pull request as draft December 11, 2023 16:10
@m-horky
Copy link
Contributor Author

m-horky commented Dec 11, 2023

Set to draft; src/cloud_what/_base_provider.py needs updating as well.

@m-horky m-horky marked this pull request as ready for review December 11, 2023 17:11
@cnsnyder cnsnyder requested a review from a team December 11, 2023 17:11
@m-horky
Copy link
Contributor Author

m-horky commented Dec 11, 2023

Question for the reviewer: would it be worth to define the colors on a single place? This code isn't updated often, and it is quite isolated, so maybe it is enough, as it has been for several years now.

Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I would define colors on single place. Maybe it could be useful for other parts of code. We could make output of subscription-manager more colorful.

msg += (
red_col
+ " https://"
+ "https://"
+ f"{normalized_host(self.host)}:{safe_int(self.ssl_port)}{handler} {request_type}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are significantly changing debug printing, then please print request_type first to get something like this:

Request:
GET https://centos8-candlepin:8443/candlepin/ (using consumer auth)

@m-horky m-horky force-pushed the mhorky/connection-debug branch from 70110fd to 7f837cc Compare December 12, 2023 13:25
@m-horky
Copy link
Contributor Author

m-horky commented Dec 12, 2023

Thanks. You are right, I discovered I even made local changes that did that, I forgot to finish it.

This is how it looks like now:

image

This change clearly labels each section that's printed to make it easier
to identify the interesting parts.

This also adds SUBMAN_DEBUG_PRINT_TRACEBACKS flag to print the traceback
directly to stdout.

GREEN:  title
RED:    method + url (request), status code (response)
BLUE:   headers
YELLOW: body
WHITE:  call stack
@m-horky m-horky force-pushed the mhorky/connection-debug branch from 7f837cc to 22e5630 Compare December 13, 2023 09:56
Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks for updates 👍

@jirihnidek jirihnidek merged commit 51f75db into main Dec 14, 2023
16 checks passed
@jirihnidek jirihnidek deleted the mhorky/connection-debug branch December 14, 2023 13:58
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.

2 participants