dbg: add dbg_printf_id

[sreimers]

No description provided.

[alfredh]

is unsigned 32-bit generic enough for all cases ?

Perhaps "string" is an option ?

In the past when I was debugging a server with hundreds of calls, we had a
session ID for each call, which was a UUID. The first 4-5 chars of the UUID was used
in the debug tag, it was usually unique enough.

[sreimers]

The problem I see with a string is that this is maybe harder to handle, the basic idea I have in mind is to add setter functions to relevant objects:

struct icem {
...
	uint32_t debug_id;
}

void icem_set_debug_id(uint32_t debug_id);

static struct ice_candpair *construct_valid_pair(struct icem *icem,
					     struct ice_candpair *cp,
					     const struct sa *mapped,
					     const struct sa *dest)
{
	uint32_t debug_id = icem->debug_id;
...

	if (!lcand) {
		DEBUG_WARNING_ID("no such local candidate: %J\n", mapped);
		return NULL;
	}
	if (!rcand) {
		DEBUG_WARNING_ID("no such remote candidate: %J\n", dest);
		return NULL;
	}
...
}

Maybe a fixed length could work?

#define DEBUG_ID_LEN 8;

struct icem {
...
	char debug_id[DEBUG_ID_LEN];
}

void icem_set_debug_id(char *debug_id);

static struct ice_candpair *construct_valid_pair(struct icem *icem,
					     struct ice_candpair *cp,
					     const struct sa *mapped,
					     const struct sa *dest)
{
	char *debug_id = icem->debug_id;
...

	if (!lcand) {
		DEBUG_WARNING_ID("no such local candidate: %J\n", mapped);
		return NULL;
	}
	if (!rcand) {
		DEBUG_WARNING_ID("no such remote candidate: %J\n", dest);
		return NULL;
	}
...
}

[alfredh]

there is also an option to implement this without changes in DBG module.

You can define a session-id string, for example SIP Call-ID or webrtc session-id.
This string is the passed manually to all relevant modules.

Each module, for example ICE, will add this string to all debug messages.

When you session is finished, you can take the logfile and grep for the session-id.

[sreimers]

One benefit is with dbg_printf_id custom log handlers for example can generate separate logfiles per call/session or if you use central logging like graylog or grafana loki, you can use the identifier as indexed key value metric, without parsing the string.

[sreimers]

I refactored the PR with struct pl instead of uint32_t. This should be more flexible.

[alfredh]

is it better to pass "debug_id" via the macro parameters, instead of assume the name ?

[sreimers]

It follows the same technique used for DEBUG_LEVEL and DEBUG_MODULE.
The compiler errors if debug_id is not defined. I think it's much easier for rewriting existing DEBUG code since you have only define debug_id and add _ID. And this needs less reformatting new lines, that was the idea behind it.

[alfredh]

Should we consider uppercase: DEBUG_ID ?

[alfredh]

as an example usecase, this ID could be used by baresip/src/call.c

with multiple calls, each call has a unique ID. Perhaps also consider options to turn it on/off ?

[alfredh]

should the id (aka "tag") be part of the log line ?

[alfredh]

const for id ?

[sreimers]

I think a custom log handler should able to call mem_ref(id), this discards the const qualifier.

[alfredh]

Should we merge this one to main, or close it ?

[alfredh]

this can probably be merged now

Consider the name "ID" or "TAG" for your change.

Another thing we should fix is to avoid double locking and formatting here:

void dbg_printf(int level, const char *fmt, ...)
{
	va_list ap;

	va_start(ap, fmt);
	dbg_vprintf(level, fmt, ap);
	va_end(ap);

	va_start(ap, fmt);
	dbg_fmt_vprintf(level, fmt, ap);
	va_end(ap);
}