RFC: Support source/severity for messages

[Lectem]

This pull request introduces some new metadata to messages: sources and severity. The main objective was to provide a way to send internal information to the user by default, such as errors related to system sampling for example.

Currently sources are User/Tracy, and severity levels are Trace, Debug, Info, Warning, Error, Fatal, based on https://arxiv.org/abs/2109.01192 .

Note that some commits are not finalized / meant to be merged as is, as I'd like some feedback first on the following questions.

1. Should we allow user-defined levels / channels?
2. Should we break the API&ABI to add this to the current TracyMessage* defines?

• If not, are tracy::MessageSourceType::User and tracy::MessageSeverity::Info the correct defaults for those?
• If yes,

3. This has already been enhanced and discussed a bit on discord, but I'd like feedback on the filters

• Version A
• Version B. Note that currently, toggling the filter requires to click the button or the text precisely

Still needs to be done:

• Display source/severity of the messages
• Adjust severity level of TracyDebug messages by splitting into multiple macros
• Update manual
• Polish code ?

Any kind of feedback is welcome.

[slomp]

Should we break the API&ABI to add this to the current TracyMessage* defines?

No, TracyMessage(msg) as is should just become the "common denominator", e.g. TracyLog(User, Info, msg), as you pointed (good enough in my opinion).

Should we allow user-defined levels / channels?

I think it's a bit premature and can lead to over-engineering. Since source and severity are enums backed by an ordinal number, we should be able to extend that in the future.

I'd like feedback on the filters

I don't have a major preference, but A is a bit more compact and feels less busy to the eyes.

What does the "Reset" button do with the filters? Will it "check-all", besides clearing the text filter? Will the severity levels also be parsed by the textual filter (fatal, error, warning, etc)?

[Lectem]

What does the "Reset" button do with the filters? Will it "check-all", besides clearing the text filter?

Currently it checks all + clear text (reset all filters). It used to be linked to the text field and was walled Clear.

Will the severity levels also be parsed by the textual filter (fatal, error, warning, etc)?

No, but I suppose we could? One thing that does bother me a little is that you cant easily toggle off all filters but one, with this suggestion this would be possible by typing the severity name. Not sure if it's the best UX though.

[slomp]

No, but I suppose we could?

Nah, I think we're good as is. Should people bring this matter in the future, we can always implement it later.

~On a different note: will this new logging interface be accessible somehow in the server itself? (see: https://github.com/wolfpld/tracy/issues/1131)~ (Just had some time to actually look at the diff, and yes, there's an "internal" enum for the source! 👍 )

[Lectem]

No, TracyMessage(msg) as is should just become the "common denominator", e.g. TracyLog(User, Info, msg), as you pointed (good enough in my opinion).

@slomp circling back on this, I'm now a bit stuck on the naming for the new macro because we currently have this:

#define TracyMessage( txt, size ) tracy::Profiler::Message( txt, size, TRACY_CALLSTACK, tracy::MessageSourceType::User, tracy::MessageSeverity::Info )
#define TracyMessageL( txt ) tracy::Profiler::Message( txt, TRACY_CALLSTACK, tracy::MessageSourceType::User, tracy::MessageSeverity::Info )
#define TracyMessageC( txt, size, color ) tracy::Profiler::MessageColor( txt, size, color, TRACY_CALLSTACK, tracy::MessageSourceType::User, tracy::MessageSeverity::Info )
#define TracyMessageLC( txt, color ) tracy::Profiler::MessageColor( txt, color, TRACY_CALLSTACK, tracy::MessageSourceType::User, tracy::MessageSeverity::Info )


#define TracyMessageS( txt, size, depth ) tracy::Profiler::Message( txt, size, depth, tracy::MessageSourceType::User, tracy::MessageSeverity::Info )
#define TracyMessageLS( txt, depth ) tracy::Profiler::Message( txt, depth, tracy::MessageSourceType::User, tracy::MessageSeverity::Info )
#define TracyMessageCS( txt, size, color, depth ) tracy::Profiler::MessageColor( txt, size, color, depth, tracy::MessageSourceType::User, tracy::MessageSeverity::Info )
#define TracyMessageLCS( txt, color, depth ) tracy::Profiler::MessageColor( txt, color, depth, tracy::MessageSourceType::User, tracy::MessageSeverity::Info )

I can't seem to find a good letter for the versions with severity levels. "S" (for severity) is already taken for the "stack depth" versions, and "L" (for level) is also used for the litterals version... This would also add 8 new defines (which is not really a problem in itself but well).

This leads me to the following questions:

• Should we keep TracyMessage* and use another letter, if yes:
  • which letter ?
  • should I use another name for the severity level ?
• Or should I simply change the name for something like TracyUserLog* when using severity params ?
  • This way we can not mistake it for something else
  • Message would be an alias for "tracy::MessageSeverity::Info"
  • What about the consequences to the UI ? Does this mean changing the rename "Message" buttons/window to "Logs" ?

[wolfpld]

This would also add 8 new defines (which is not really a problem in itself but well).

The exploding number of macro combinations is actually a problem.

[slomp]

In my opinion, we should introduce just one new "overarching" macro to the public API:

#define TracyLogString(level, string, length, color, callstack_depth) ...

and leave further macroing and abstractions entirely up to the user.

[Lectem]

In my opinion, we should introduce just one new "overarching" macro to the public API:

#define TracyLogString(level, string, length, color, callstack_depth) ...

I considered that too to limit the number of potential macros, but I think we need a way to at least differentiate between literal and non-literal to avoid unnecessary copies? or we could use some size hint for litterals such that we could check for it (for example if(length==-1) { /*litteral*/}, but that would be introducing a new semantic, I don't like the idea very much since everywhere else we have either string, size or just plain string params).

The exploding number of macro combinations is actually a problem.

I'd very much like to avoid it too. If we were to break compat, I would simply expose color+source+severity as a single param "metadata". I'm not sure I would even expose all the depth stack variants (I tend to think that if someone want to customize depth of callstack captures, they can just have their own macros).

So unless we break the current API (which we most likely do not want), or limit the number of macros to not expose every variant (but then we expose ourselves to questions such as "why variant XYZ does not exist), I think the best it to go the "on definition that covers all use-cases, users will create their own wrapper" such as suggested above is the best way to go.

So to sum up I'm thinking of either:

#define TracyLogString(sevlevel, txt, length, color, callstack_depth) /*impl*/
#define TracyLogStringL(sevlevel, txt, length, color, callstack_depth) /*impl*/

or

// Litterals can use length == -1
#define TracyLogString(sevlevel, txt, length, color, callstack_depth) /*impl*/

The only other solution I have in mind would be to use variadics

#define TracyLogString(sevlevel, ...) tracy::Profiler::Message( tracy::MessageSourceType::User, sevlevel, __VA_ARGS__ )

and let the overload resolution of tracy::Profiler::Message take over.