Text logger

[figueroa1395]

This PR implements a text logger in the core with multithreaded capability. Unit tests are added.

[figueroa1395]

Tests and probably implementation of the multithreaded text logger with flush handler are not yet in place, hence I put this as a draft. The rest can already be reviewed.

Any input is already appreciated, specially regarding the flush handler (which we don't have to expose yet, but I thought I'd be nice to add).

[figueroa1395]

I have several TODOs that are meant for me - and the reviewers - to consider before merging. Most of the implementation should now work.

What's missing:

• Fix the benchmark and run it with the new interface
• Add an "end-to-end" test to proof the text logger actually logs everything we currently have in the main model.

[figueroa1395]

Benchmark works fine locally. @mgovers could you run it in windows and WLS from this branch locally to confirm please.

Only missing thing is to add the "end-to-end" test.

Btw, since the text logger adds a timestamp already, given the fact the we use an internal timer already (in current implementation), we would have a "double" timestamp. I went with this decision because I'm unsure if everything will go via the timer in the future, and the timer itself doesn't give us a timestamp like we would like it for the text logger. This is definitely something to look at but I consider it out of scope for when we want to polish the loggers. What do you think?

[mgovers]

please double-check whether the calculation info still works correctly

[figueroa1395]

This works, but now the user is responsible for cleaning between different calculations (if desired) if the same logger is used. Hence the benchmark has been adjusted to this.

[mgovers]

Benchmark works fine locally. @mgovers could you run it in windows and WLS from this branch locally to confirm please.

Done. Both Windows and WSL pass

[mgovers]

can you explain why you went for std::invoke? not against it, just interested

[figueroa1395]

No particular reason. Doing fn() should suffice here as I require it to be std::invocable, but I guess I went with the more "generic" option.

[mgovers]

so what happens when it's of the form

MultiThreadedLoggerImpl other;
MultiThreadedLoggerImpl current{other, some_other_arg};

?
does it need to be explicitly deleted?

[figueroa1395]

It doesn't need to be explicitly deleted because in MultiThreadedLoggerImpl current{other, some_other_arg} the first argument is also of type MultiThreadedLoggerImpl, which the concept forbids. This way, a MultiThreadedTextLogger recognizes a constructor with a FlushHandler present, but it prevents this constructor from triggering instead of the copy constructor during overload resolution. For instance, it allows:

MultiThreadedLoggerImpl logger{flush_handler};
MultiThreadedLoggerImpl copy{logger};

[mgovers]

isn't there a direct formatted std::chrono? or is that C++26?

[figueroa1395]

This is the current way as far as I know (cfr. https://en.cppreference.com/w/cpp/chrono/system_clock/formatter.html).

[mgovers]

prevents unneeded copy

Suggested change

	destination.data_ << data_.str();
	destination.data_ << data_.view();

[figueroa1395]

The copy is needed here. destination.data_ << data_.view(); would provide a string_view which can dangle.

For example, consider the user provided logger and a batch calculation with multithreading enabled. If we use a view here, once the child threads are done with the calculation, they merge_into the main logger and then they are deleted. That would be a problem.

[figueroa1395]

@mgovers @nitbharambe Should we already make this something like

Suggested change

	template <LazyLoggingFn Fn> void log(LogEvent tag, Fn&& fn) { log(tag, std::invoke(std::forward<Fn>(fn))); }
	template <LazyLoggingFn Fn> void log(Fn&& fn) { log(LogEvent::unknown, std::invoke(std::forward<Fn>(fn))); }
	template <LazyLoggingFn Fn> void log(LogEvent tag, Fn&& fn, bool invoke = false ) {
	if (!invoke) { return; }
	log(tag, std::invoke(std::forward<Fn>(fn))); }
	template <LazyLoggingFn Fn> void log(Fn&& fn, , bool invoke = false ) {
	if (!invoke) { return; }
	log(LogEvent::unknown, std::invoke(std::forward<Fn>(fn))); }

to make it truly lazy already?

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
97.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud