[c++20] Simple-Log - Feedback gewünscht



  • Habs jetzt btw mal mit FetchContent ausprobiert:

    cmake_minimum_required(VERSION 3.14 FATAL_ERROR)
    
    project(test_cmake)
    
    include(FetchContent)
    
    FetchContent_Declare(
    	simple_log
    	GIT_REPOSITORY https://github.com/DNKpp/Simple-Log
    	GIT_TAG     origin/improve_cmake
    )
    FetchContent_MakeAvailable(simple_log)
    
    add_executable(
    	${PROJECT_NAME}
    	main.cpp
    )
    
    target_link_libraries(
    	${PROJECT_NAME}
    	PUBLIC simple_log
    )
    

    Das hat jetzt ohne Probleme funktioniert und auch mein include dir richtig gesetzt. Was sollte denn der alias beheben?



  • Das alias target ist nicht notwendig, nur hilfreich 😉

    Ein Alias definiert man eig. dann immer als mit einem Art Namespace. Sehr verbreitet ist z.B. Projektnamen::TargetName. In deinem Fall ist das jetzt beides identisch, ist aber okay, sieht man auch bei vielen Libs bzw. Packages von CMake selbst (z.B. Sqlite3).
    Also wäre dein Alias Name z.B. "simple_log::simple_log"

    Es gibt ein paar Ideen dahinter:

    • Namensdopplungen vermeiden. Ist wie bei C++ mit nem zusätzlichen Namespace natürlich einfach unwahrscheinlicher.
    • Es eindeutig als CMake Target definieren. Die Anweisung target_link_libraries nimmt nicht nur CMake Targets, sondern eben auch "normale" lib Namen. Also wenn du da jetzt "abc" hinschreibst, dann sucht CMake zunächst nach einem CMake Target abc und wenn es das nicht findet, sucht es nach einer Lib z.B. /usr/lib/libabc.so. Wenn es das auch nicht findet, kriegst du halt eine Fehlermeldung, dass er nix gefunden hat. Hat der Name aber ein "::" drin, weiß CMake das es nur ein Target sein kann. Das widerum hat mehrere Vorteile:
      • Die Gefahr sinkt, dass er das CMake Target nicht findet, aber zufällig ne lib global installiert findet, die halt genauso heißt (Das ist mir schon öfters mal passiert, insbesondere wenn ich die Lib z.B. zu Testzwecken schon mal bei mir auf dem System irgendwann installiert habe)
      • Es ist selbstsprechender. Wer es liest, weiß direkt: Ah ein CMake Target
      • Wenn er das CMake Target nicht findest, kriegst du eine weniger generische Fehlermeldung 😉
    • Du kannst deine Targets konsistent in der Nutzung machen. Für install / export Logik gibt es auch Namespaces etc. ... man kann das dann so machen, dass der Lib Name für den Anwender immer gleich ist, egal ob per FetchContent, lokal ins Projekt kopiert, auf dem System installiert und mit findPackage etc.
    • Es ist irgendwie ein Standard 😃

    Also nicht super schlimm, wenn man es nicht hat. Auf der anderen Seite gibt es ja doch schon ein paar kleinere Vorteile. Insbesondere der "Es ist Standard" Aspekt wiegt schon recht stark finde ich. Grade CMake würde definitiv einfacher in der Nutzung sein, wenn nicht jeder sein eigenes Süppchen kochen würde. Und da es nur eine Mehrzeile ist, lohnt sich das imo definitiv.



  • Nebenbei: Du könntest auch einen "examples" Ordner hinzufügen mit minimalen Beispiel deiner Lib und Verwendung in CMake.
    Viele Leute tun sich echt schwer mit CMake und ich glaube so ein richtiges funktionierendes Beispiel wie man die Lib mit FetchContent, findPackage etc. nutzt ist sehr hilfreich. (Im besten Fall verwendet man dort auch best Practices, das übernehmen die Leute dann auch, z.B. immer schön die target Befehle mit PUBLIC / PRIVATE / INTERFACE setzen). Man spart sich dann auch die Mühe in der Dokumentation und kann einfach sagen: Unterstützt diese Arten eingebunden zu werden, hier link zu den Beispielen.
    Und du hast praktischerweise auch immer direkt Test Code, um zu sehen, ob alles nocht geht, wenn du an deinem CMake Code rumgebastelt hast 😃



  • @Leon0402 sagte in [c++20] Simple-Log - Feedback gewünscht:

    Viele Leute tun sich echt schwer mit CMake und ich glaube so ein richtiges funktionierendes Beispiel wie man die Lib mit FetchContent, findPackage etc. nutzt ist sehr hilfreich.

    Dazu müsste ich tatsächlich selbst erstmal wissen, wie das funktioniert 😃

    @Leon0402 sagte in [c++20] Simple-Log - Feedback gewünscht:

    Im besten Fall verwendet man dort auch best Practices, das übernehmen die Leute dann auch, z.B. immer schön die target Befehle mit PUBLIC / PRIVATE / INTERFACE setzen). Man spart sich dann auch die Mühe in der Dokumentation und kann einfach sagen: Unterstützt diese Arten eingebunden zu werden, hier link zu den Beispielen.

    Auch die müsste ich dazu erstmal beherrschen, jemanden haben der die für mich umsetzt oder mir beibringt.
    Aber ich habe ja bereits einen examples ordner. Zugegeben, der ist ein bisschen versteckt unter src/examples.
    https://github.com/DNKpp/Simple-Log/tree/master/src/examples

    Hab heute allgemein schon ein bisschen rum geschaufelt. Entspricht das nun eher dem, was du erwarten würdest?



  • Ja, das sieht schon besser aus!

    CMakeLists.txt

    project(simple_log)
    

    Willst du keine Versionsnummer haben?

    set(TestsDir "${CMAKE_CURRENT_SOURCE_DIR}/src/tests/")
    set(ExamplesDir "${CMAKE_CURRENT_SOURCE_DIR}/src/examples/")
    

    Das würde ich mir persönlich sparen, einfach direkt an die entsprechende Stelle einsetzen.

    add_library(${PROJECT_NAME} INTERFACE)
    

    Man sollte eher vermeiden den Projektnamen als Target Namen zu nehmen. Lieber gleich das ganze seperat halten. Wenn ein Projekt wächst ist es auch nicht unüblich, dass man mehrere Targets hat.

    Für deine anderen CMakeFiles:

    cmake_minimum_required(VERSION 3.14 FATAL_ERROR)
    
    project(simple_log_examples)
    

    Nicht nochmal wiederholen, das brauchst du nicht.

    add_executable(
    	${PROJECT_NAME}
    	main.cpp
    )
    
    target_link_libraries(
    	${PROJECT_NAME}
    	PRIVATE
    	simple_log
    )
    

    Entsprechend hier dann auch eigene Namen wählen und direkt einsetzen. Nicht da project() misbrauchen.

    Für deine Tests:

    FetchContent_MakeAvailable(catch2)
    list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/contrib)
    include(Catch)
    

    Das ist vlt. ganz hilfreich. FetchContent_MakeAvailable kannst du so oft aufrufen wie du willst, es wird nur einmal gedownloaded beim anderen mal werden nur die Variablen gesetzt (catch2_SOURCE_DIR). Mit dem Snippet oben kannst du CMake Helper von Catch hinzufügen. Damit kriegst du z.B. die Funktion catch_discover_tests, siehe doku von Catch.

    add_executable(
    	${PROJECT_NAME}
    	main.cpp
    	BaseRecordTests.cpp
    	TupleAlgorithmTests.cpp
    	PredicateTests.cpp
    	FilterTests.cpp
    	RecordQueueTests.cpp
    	StringPatternTest.cpp
    	BasicSinkTests.cpp
    	FlushPolicyTests.cpp
    	#LogTest.cpp
    )
    

    Ich persönlich bin hier ein Freund von target_sources, um die Dateien in dem Verzeichnis hinzuzufügen, wo sie auch liegen. Das ist zumindest etwas flexibler, aber größtenteils vermutlich geschmackssache. Wollte nur mal gesagt haben, dass es das gibt und sehr praktisch ist 😉

    Ich habe auch mal in den Test Code reingeguckt an sich, weil ich mich grade auch mit Tests beschäftigt 😃 Das erste, was mir aufgefallen ist, dass du ein ähnliches Problem mit dem benennen hast. Du verwendest ja auch das GIVEN-WHEN-THEN. Und ich finde es cool, dass man da quasi seinen Test Code direkt mit dokumentieren kann, aber manchmal fällt mir das sinnvolle Namen geben schwer. Zum Beispiel dein Konstuktur Test.

    WHEN("using std::string as message type") THEN("is empty")

    Ist ja schon irgendwie nicht so doll. Aber was besseres fällt mir halt auch nicht ein 😃 Gefühlt will man da sowas nicht aussagendes hinschreiben wie "Wenn man das Objekt konstruiert, dann ist es korrekt konstruiert" ... ja wow 😃

    Bei deinen Beispielen ist mir aufgefallen, dass sie relativ komplex / lang sind, also selbst das einfachste. Hier hatte ich vlt. überlegt, ob es Sinn ergibt, wenn du diese Factory Methode als Teil der Lib machst? Das würde die Länge / Komplexität denke ich relativ stark verringern, ggf. auch mit templates.
    In komplexeren Fällen kann man ja evtl. im nachinein gewisse Sachen verwenden oder dann doch eben die Factory Methode selbst implementieren?

    inline logging::Logger_t gLog{ *gLoggingCore, logging::SeverityLevel::info };
    

    Hier stört mich etwas, dass man direkt den enum wert verwendet, während man später immer logging::SetSev::debug verwenden muss. Kann man hier nicht vlt. überall das gleiche verwenden, also vermutlich ddas zweite (von der Konstruktion her, die dahinter steckt), aber vlt. mit dem Namen des ersten (da etwas klarer)



  • Nachdem ich nun die letzten beiden Wochen praktisch fast jede freie Minute an der Lib gearbeitet habe, habe ich mich nun dazu entschlossen den ersten "offiziellen" github alpha-release zu erstellen. Seit dem letzten Post sind einige neue UnitTests entstanden, Bugs gefixt und ein kompletter ReadyToGo-Header erstellt worden. Durch das c++17 inline Variable-Feature ist es ja sehr einfach geworden, globals nur auf Wunsch der User zu erstellen. D.h. wenn jemand den Header included ist er "Ready2Go" und kann direkt loslegen, erhält dafür aber eben auch drei, direkt von mir defininierte, globales. Alle anderen includieren einfach nicht.

    Nebenbei ist eine komplette FlushPolicy property entstanden, die entscheidet ob der Stream des Sinks nun geflushed werden soll oder nicht. Es ist kein riesen Aspekt aber dafür highly customizable (hey, Schlagwort 😃 ).

    Ich bin weiterhin über Feedback und Anregungen aller Art.

    MfG

    PS: Vielen Dank übrigens @Leon0402 und auch @Zhavok , die hier und da mich immer wieder mit ihrer Unwissenheit dazu bringen, das Ganze doch noch ein wenig zugänglicher zu machen. Aber jetzt ist Schluss damit 😃



  • @DNKpp sagte in [c++20] Simple-Log - Feedback gewünscht:

    Vielen Dank übrigens @Leon0402 und auch @Zhavok , die hier und da mich immer wieder mit ihrer Unwissenheit dazu bringen, das Ganze doch noch ein wenig zugänglicher zu machen. Aber jetzt ist Schluss damit

    Na da hast du ja wieder schöne Worte gefunden. Da hat man richtig Lust dir zu helfen^^ Zur Richtigstellung aber: Ob ich deinen (Beispiel) Code verstehe oder (unnötig) komplex finde sind im Zweifelsfall komplett unterschiedliche Sachen 😉
    So wie ich auch deinen funktionierenden ursprünglichen CMake Code verstanden habe, aber ihn trotzdem nicht gut fand. "Funktioniert" ist nicht das Maß aller Dinge.



  • @DNKpp sagte in [c++20] Simple-Log - Feedback gewünscht:

    log() << "Hallo," << " Welt" << "!"; // log stellt hier eine Logger Instanz dar. Der Operator () erstellt eine RecordBuilder Instanz, die über die << Ops weiter gereicht wird.

    Das sieht teuer aus. Weitergereicht wird so ja immer, egal ob geloggt wird oder nicht (log-level).
    Boost löst das meines Wissens über ein Macro. Nicht schön, aber wenn ich nichts loggen will, dann soll das nichts tun aber auch möglichst billig sein.



  • @Leon0402 sagte in [c++20] Simple-Log - Feedback gewünscht:

    @DNKpp sagte in [c++20] Simple-Log - Feedback gewünscht:

    Vielen Dank übrigens @Leon0402 und auch @Zhavok , die hier und da mich immer wieder mit ihrer Unwissenheit dazu bringen, das Ganze doch noch ein wenig zugänglicher zu machen. Aber jetzt ist Schluss damit

    Na da hast du ja wieder schöne Worte gefunden. Da hat man richtig Lust dir zu helfen^^ Zur Richtigstellung aber: Ob ich deinen (Beispiel) Code verstehe oder (unnötig) komplex finde sind im Zweifelsfall komplett unterschiedliche Sachen 😉
    So wie ich auch deinen funktionierenden ursprünglichen CMake Code verstanden habe, aber ihn trotzdem nicht gut fand. "Funktioniert" ist nicht das Maß aller Dinge.

    Passt schon, brauchst dich nicht zu verteidigen. Bin dir trotzdem dankbar für dein Feedback 😃

    @Jockelx sagte in [c++20] Simple-Log - Feedback gewünscht:

    @DNKpp sagte in [c++20] Simple-Log - Feedback gewünscht:

    log() << "Hallo," << " Welt" << "!"; // log stellt hier eine Logger Instanz dar. Der Operator () erstellt eine RecordBuilder Instanz, die über die << Ops weiter gereicht wird.

    Das sieht teuer aus. Weitergereicht wird so ja immer, egal ob geloggt wird oder nicht (log-level).
    Boost löst das meines Wissens über ein Macro. Nicht schön, aber wenn ich nichts loggen will, dann soll das nichts tun aber auch möglichst billig sein.

    Ich kann diese Argumentation in soweit nachvollziehen, dass sie sich wahrscheinlich auf den debug level beschränkt. Hier hatte ich in der Tat bereits über eine Lösung nachgedacht. Bisher bin ich leider noch dazu gekommen, da irgendetwas draus zu machen. Zusätzlich ist das Problem, dass ich die Nutzer ja nicht auf meine erstellten SeverityLevel festnageln möchte, sondern sie durchaus auch eigene nutzen können. Ist jedenfalls nicht ganz trivial.
    Abseits des debug Mode Arguments sehe ich den Grund allerdings nicht. Nachrichten werden abgesetzt, wenn es etwas an der derzeitigen Position zu berichten gibt, ganz unabhängig davon, ob das nun irgendwo in einer Konsole ausgegeben, in eine Datei gespeichert oder per Netzwerk verschickt werden soll. D.h. an dieser Stelle ist nicht klar, ob und von wem die Nachricht eigentlich verarbeitet werden soll. Daher lässt sich aus meiner Sicht auch keine sinnvolle Optimierung erstellen. Auch die boost Makros werden an dieser Stelle keine schwarze Magie besitzten, außer eben den genannten debug Mode zu no-open.



  • @DNKpp sagte in [c++20] Simple-Log - Feedback gewünscht:

    Zusätzlich ist das Problem, dass ich die Nutzer ja nicht auf meine erstellten SeverityLevel festnageln möchte, sondern sie durchaus auch eigene nutzen können. Ist jedenfalls nicht ganz trivial.

    Was hat das eine mit dem anderen zu tun?

    @DNKpp sagte in [c++20] Simple-Log - Feedback gewünscht:

    Auch die boost Makros werden an dieser Stelle keine schwarze Magie besitzten, außer eben den genannten debug Mode zu no-open.

    Doch, sofern ich es jetzt nicht komplett falsch verstehe:
    https://www.boost.org/doc/libs/1_66_0/libs/log/doc/html/log/tutorial/trivial_filtering.html

    Important
    
    Remember that the streaming expression is only executed if the record passed filtering. Don't specify business-critical calls in the streaming expression, as these calls may not get invoked if the record is filtered away. 
    


  • @Jockelx Ich hatte eigentlich eher erwartet, dass du jetzt tatsächlich auf NoOps hinaus wolltest und nicht auf Filter Mechaniken. Filtern ist in meiner Lib auf Sink lvl implementiert und wird dann von dem Sink auch nicht mehr weiter verwertet und geskipped.



  • @DNKpp
    Ja, ist mir klar, trotzdem wird ja erstmal immer alles weggestremed und dann ggf. verworfen.
    Wir verwenden bei uns boost, kapseln das allerdings vorher und dieses vorher kapseln sieht im Prinzip so aus wie bei dir. Und es macht einfach leider einen Performanceunterschied aus. Haben aber auch eine Lösung dafür.



  • @Jockelx Jep, das glaube ich! Ist auch logisch. Ich schreib mir das mal auf und denke darüber nach. Vielen Dank für deinen Input.

    EDIT:
    Ansonsten wäre es auch denkbar das bereits auf Logger lvl abzufangen. Wirkliche Filter wollte ich in Core eigentlich nicht einbauen.



  • @DNKpp sagte in [c++20] Simple-Log - Feedback gewünscht:

    @Jockelx Jep, das glaube ich! Ist auch logisch. Ich schreib mir das mal auf und denke darüber nach. Vielen Dank für deinen Input.

    EDIT:
    Ansonsten wäre es auch denkbar das bereits auf Logger lvl abzufangen. Wirkliche Filter wollte ich in Core eigentlich nicht einbauen.

    Ich denke annähernd Zero Overhead da lässt im Bedarfsfall machen, wenn z.B. der Ausdruck gLog() << logging::SetSev::debug im Release-Fall (bzw. wenn der Filter nicht greift) einen Null-Logger/Sink zurückgibt, bei dem die Log-Operationen einfach leere Funktionen sind, die letzendlich wegoptimiert werden.

    Dazu müsste Das Log-Level allerdings m.E. eine Compile-Time-Konstante sein, so dass das analog zu z.B. gLog<Severity::debug>() << ... funktioniert. Dann würden die Log-Level aber direkt einkompiliert und könnten nicht mehr programmatisch bestimmt werden - falls das wichtig ist. So wie ich deinen derzeitigen Code verstehe, muss für jeden Log-Aufruf derzeit mindestens einen Vergleich mit dem aktiven Log-Level gemacht werden, auch wenn im weiteren nichts mehr passiert.

    Allerdings hätte man auch mit so einer Lösung das Problem, dass die Ausdrücke, die in den Log-Aufrufen auftreten immer noch ausgewertet werden, inklusive Performance beeinflussender Nebeneffekte. Ein log() << "Text" würde da bei einem operator<<(const std::string&) immer noch einen temporären std::string aus dem char*-Literal konstruieren, auch wenn log() eine leere Funktion wäre. Diesem speziellen Fall könnte man aber evtl. mit einem operator<<(const T&) {} begegnen, der auch einen char* so wie er ist fressen würde, ohne ein temporäres Objekt zu erzeugen.

    Ich befürchte aber auch, dass man wirklichen Zero Overhead nur mit Makros hinbekommt, da sie m.E. die einzige Möglichkeit sind, Ausdrücke in Logger-Argumenten nur bedingt auszuwerten.

    Man bräuchte sowas wie:

    if constexpr (debug >= loglevel)
        log() << "Message: " << get_expensive_data_with_side_effects();
    

    Allerdings so, dass man es schön in einen einzeiligen Log-Aufruf packen kann ohne jedesmal dieses if constexpr-Gefummel schreiben zu müssen.

    Edit: Noch so ne fixe Idee am Rande, auch wenn's vielleicht etwas abschweift:

    #define log(loglevel) if constexpr (loglevel >= current_logger.loglevel()) current_logger
    
    log(debug) << "Message: " << get_expensive_data_with_side_effects();
    

    Ja, leider ein Makro, aber in der Anwendung merkt man kaum, dass es eins ist, da man trotzdem alle syntaktischen Freiheiten hat, die C++ einem bietet (Log-Messages nicht als Makro-Argumente). Könnte mir vorstellen, dass Boost das vielleicht ähnlich macht. Das wäre so tatsächlich Zero Ovehead wenn man mit compile-time-konstanten "Severities" arbeitet.



  • @Finnegan Hey, vielen Dank für deine Antwort. Die severity lvl sollte ja zu 99% zur Compiletime feststehen (habe zumindest noch nie erlebt, dass man das zur Runtime irgendwie bestimmen möchte. Vll ein guter Zeitpunkt, sich mal mit consteval zu beschäftigen.
    Alles in Allem bin ich nun nach zwei Tagen hin und her überlegen zu dem Schluss gekommen, dass ich
    a. eine alternative Logger Implementierung anbieten werde, die eben filtern kann
    b. Stand jetzt, eine separate Logger Klasse anbieten möchte, der eben von sich aus im Release Mode, zumindest fast, zur Noop wird. Das ist dann natürlich vom Severity Lvl entkoppelt und an den Build Modus gebunden. Wer etwas granulareres benötigt, der ist jederzeit in der Lage sich einen eigenen Logger zu bauen und dadurch auf exakt seine Bedürfnisse anzupassen.


Anmelden zum Antworten