Skip to content

Comments

feat: improve notification log #4735#4737

Open
tzzed wants to merge 3 commits intotelefonicaid:masterfrom
tzzed:feat/4735_improve_notification_log
Open

feat: improve notification log #4735#4737
tzzed wants to merge 3 commits intotelefonicaid:masterfrom
tzzed:feat/4735_improve_notification_log

Conversation

@tzzed
Copy link
Contributor

@tzzed tzzed commented Dec 11, 2025

Fix #4735.
Improves notification error logging by including the response body when a notification fails (HTTP response is not OK). This enhancement provides better visibility into notification failures by showing not only the HTTP status code but also the actual error message returned by the endpoint.

Changes

Modified httpRequestSend.cpp:661-662 to include the response body (outP) in the warning log message when notification responses are NOT OK
The log message now shows: notification ID, HTTP code, and the complete response body

Benefits

Easier debugging of notification failures by having the full error context in logs
No need to enable debug mode or use external tools to see why a notification failed
Aligns with the information already logged for successful notifications

@fgalan
Copy link
Member

fgalan commented Dec 12, 2025

Thanks for the contribution!

Can you show how an actual log looks like after your modification?

@tzzed
Copy link
Contributor Author

tzzed commented Dec 12, 2025

@fgalan sure! here an exemple of error response. The body can be parsed.

Notification (subId: 693c6da61cf96df42d01b62a) response NOT OK, http code: 400, response body: HTTP/1.1 400 Bad Request

Date: Fri, 12 Dec 2025 19:32:07 GMT

Fiware-Correlator: 3eda5040-d791-11f0-bbc7-120e2b33566b; cbnotif=1

Content-Type: application/json

Content-Length: 97


{"error":"BadRequest","description":"Service not found. Check your URL as probably it is wrong."}

@tzzed tzzed force-pushed the feat/4735_improve_notification_log branch from 6ecd0e1 to f5ea5e6 Compare December 13, 2025 13:22
@tzzed
Copy link
Contributor Author

tzzed commented Dec 13, 2025

after the last commit with get_body()

time=2025-12-13T13:23:23.378Z | lvl=WARN | corr=e65bcd7c-d826-11f0-959a-7adbb77a18e0; cbnotif=1 | trans=1765632202-900-00000000004 | from=127.0.0.1 | srv=<none> | subsrv=/ | comp=Orion | op=httpRequestSend.cpp[668]:httpRequestSend | msg=Notification (subId: 693d68cb37b39bdce507608a) response NOT OK, http code: 400, response body: {"error":"BadRequest","description":"Service not found. Check your URL as probably it is wrong."}

@tzzed tzzed changed the title feat: improve notification log feat: improve notification log #4735 Dec 13, 2025
@fgalan
Copy link
Member

fgalan commented Dec 15, 2025

The log message generated shown above is fine. However, we tend to use C approaches for low-level logic (as the one related with logs). Note that using C++ functions like std::find() and std::substr() can be expensive.

I'd suggest to have a look to the functions in logTracing.cpp and create a logWarnHttpNotification() function for printing this log message, using the same way (including payload truncation, using truncatePayload(), etc.).

*/
static char* truncatePayload(const char* payload)
{
static char* truncatePayload(const char* payload) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
static char* truncatePayload(const char* payload) {
static char* truncatePayload(const char* payload)
{

Coding style: M8 rule

Comment on lines +80 to +100
void logWarnHttpNotification
(
const char* idStringForLogs,
long long httpCode,
const char* responseBody
)
{
if (responseBody == NULL)
{
responseBody = "";
}

char* effectiveBody = get_body(responseBody);
LM_W(("Notification (%s) response NOT OK, http code: %d, response body: %s",
idStringForLogs, httpCode, effectiveBody));

if (effectiveBody)
{
free(effectiveBody);
}
}
Copy link
Member

@fgalan fgalan Dec 16, 2025

Choose a reason for hiding this comment

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

The logInfoPayloadMaxSize limit should be taken into account. No payload in log traces should go beyond that limit.

I'd suggest to use something like in other functions in this file

  bool cleanAfterUse = false;
  char* effectivePayload;

  if (strlen(payload) > logInfoPayloadMaxSize)
  {
    effectivePayload = truncatePayload(payload);
    cleanAfterUse = true;
  }
  else
  {
    effectivePayload = (char*) payload;
  }

If resquestBody also include headers, etc. and you want to skip that maybe something like this could be useful (please check, I haven't checked myself)

char* payload = strchr(resquestBody, '{');

Comment on lines +56 to +72
static char *get_body(const char *response) {
const char *start = strchr(response, '{');
if (!start) return NULL;

const char *end = strrchr(start, '}');
if (!end || end < start) return NULL;

size_t len = (size_t)(end - start + 1);

char *out = (char*)malloc(len + 1);
if (!out) return NULL;

memcpy(out, start, len);
out[len] = '\0';

return out;
}
Copy link
Member

Choose a reason for hiding this comment

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

Probably not needed taking into account what I comment at #4737 (comment)

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.

Improve Orion Notification Error Logs (Missing Root Cause)

2 participants