From 72cccb0f1ea1e0b3a35691d15f6d019df53c81f1 Mon Sep 17 00:00:00 2001 From: Dale-Koenig Date: Fri, 1 Mar 2024 19:12:34 +0900 Subject: [PATCH 1/5] Expand interface to support trace links --- alica_engine/include/engine/IAlicaTrace.h | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/alica_engine/include/engine/IAlicaTrace.h b/alica_engine/include/engine/IAlicaTrace.h index 62b9a568e..ae82e0cab 100644 --- a/alica_engine/include/engine/IAlicaTrace.h +++ b/alica_engine/include/engine/IAlicaTrace.h @@ -8,6 +8,7 @@ #include #include #include +#include namespace alica { @@ -57,10 +58,24 @@ class IAlicaTrace } }; +class SpanLink +{ +public: + std::string context; + // Warning: Ensure string_views in attributes are valid for the lifetime of the span link + std::unordered_map attributes; +}; + class IAlicaTraceFactory { public: - virtual std::unique_ptr create(const std::string& opName, std::optional parent = std::nullopt) const = 0; + // Two argument version of create. Deprecated. + virtual std::unique_ptr create(const std::string& opName, std::optional parent = std::nullopt) const { return nullptr; } + virtual std::unique_ptr create(const std::string& opName, std::optional parent_context, const std::vector&) const + { + // Concrete implementation provided for backwards compatibility + return nullptr; + } virtual void setGlobalContext(const std::string& globalContext) = 0; virtual void unsetGlobalContext() = 0; virtual ~IAlicaTraceFactory() = default; From 21295fa98378e1c58dbe8e482f1ae350ae1d18ce Mon Sep 17 00:00:00 2001 From: Dale-Koenig Date: Wed, 6 Mar 2024 14:56:55 +0900 Subject: [PATCH 2/5] Allow returning context with links --- alica_engine/include/engine/BasicBehaviour.h | 11 +++++++++-- alica_engine/include/engine/BasicPlan.h | 10 +++++++++- alica_engine/include/engine/IAlicaTrace.h | 16 ++++++++++++---- alica_engine/include/engine/RunnableObject.h | 7 ++++--- alica_engine/src/engine/RunnableObject.cpp | 2 +- 5 files changed, 35 insertions(+), 11 deletions(-) diff --git a/alica_engine/include/engine/BasicBehaviour.h b/alica_engine/include/engine/BasicBehaviour.h index 6b2a28853..6bb9f2e40 100644 --- a/alica_engine/include/engine/BasicBehaviour.h +++ b/alica_engine/include/engine/BasicBehaviour.h @@ -84,7 +84,7 @@ class BasicBehaviour : private RunnableObject AgentId getOwnId() const; - void setTracing(TracingType type, std::function(const BasicBehaviour*)> customTraceContextGetter = {}) + void setTracing(TracingType type, std::function customTraceContextGetter = {}) { if (customTraceContextGetter) { RunnableObject::setTracing( @@ -93,7 +93,14 @@ class BasicBehaviour : private RunnableObject RunnableObject::setTracing(type, {}); } } - + void setTracing(TracingType type, std::function(const BasicBehaviour*)> customTraceContextGetter) + { + setTracing(type, [this, inner = std::move(customTraceContextGetter)](const BasicBehaviour*) { + tracing::SpanStartOptions options; + options.parentContext = inner(this); + return options; + }); + } void setSuccess(); void setFailure(); diff --git a/alica_engine/include/engine/BasicPlan.h b/alica_engine/include/engine/BasicPlan.h index 23f398d4f..e2ee532d6 100644 --- a/alica_engine/include/engine/BasicPlan.h +++ b/alica_engine/include/engine/BasicPlan.h @@ -53,7 +53,7 @@ class BasicPlan : private RunnableObject protected: using RunnableObject::getTraceFactory; - void setTracing(TracingType type, std::function(const BasicPlan*)> customTraceContextGetter = {}) + void setTracing(TracingType type, std::function customTraceContextGetter = {}) { if (customTraceContextGetter) { RunnableObject::setTracing( @@ -62,6 +62,14 @@ class BasicPlan : private RunnableObject RunnableObject::setTracing(type, {}); } } + void setTracing(TracingType type, std::function(const BasicPlan*)> customTraceContextGetter) + { + setTracing(type, [this, inner = std::move(customTraceContextGetter)](const BasicPlan*) -> tracing::SpanStartOptions { + tracing::SpanStartOptions options; + options.parentContext = inner(this); + return options; + }); + } virtual void onInit(){}; virtual void run(){}; diff --git a/alica_engine/include/engine/IAlicaTrace.h b/alica_engine/include/engine/IAlicaTrace.h index ae82e0cab..44dec187d 100644 --- a/alica_engine/include/engine/IAlicaTrace.h +++ b/alica_engine/include/engine/IAlicaTrace.h @@ -58,23 +58,31 @@ class IAlicaTrace } }; -class SpanLink +namespace tracing +{ +struct SpanLink { -public: std::string context; // Warning: Ensure string_views in attributes are valid for the lifetime of the span link std::unordered_map attributes; }; +struct SpanStartOptions +{ + std::optional parentContext; + std::vector links; +}; +} // namespace tracing + class IAlicaTraceFactory { public: // Two argument version of create. Deprecated. virtual std::unique_ptr create(const std::string& opName, std::optional parent = std::nullopt) const { return nullptr; } - virtual std::unique_ptr create(const std::string& opName, std::optional parent_context, const std::vector&) const + virtual std::unique_ptr create(const std::string& opName, const tracing::SpanStartOptions& options) const { // Concrete implementation provided for backwards compatibility - return nullptr; + return create(opName, options.parentContext); } virtual void setGlobalContext(const std::string& globalContext) = 0; virtual void unsetGlobalContext() = 0; diff --git a/alica_engine/include/engine/RunnableObject.h b/alica_engine/include/engine/RunnableObject.h index e44893716..8541f008a 100644 --- a/alica_engine/include/engine/RunnableObject.h +++ b/alica_engine/include/engine/RunnableObject.h @@ -58,7 +58,7 @@ class TraceRunnableObject // Set the tracing type for this runnable object. customTraceContextGetter is required for custom tracing // & this method will be called to get the parent trace context before initialiseParameters is called - void setTracing(TracingType type, std::function()> customTraceContextGetter = {}); + void setTracing(TracingType type, std::function customTraceContextGetter = {}); void setupTraceContext(const std::string& name, RunningPlan* rp); void cleanupTraceContext(); void traceInitCall(); @@ -72,7 +72,7 @@ class TraceRunnableObject TracingType _tracingType; bool _runTraced; // True if the behaviour/plan's run method has already been logged in the trace const IAlicaTraceFactory* _tf; - std::function()> _customTraceContextGetter; + std::function _customTraceContextGetter; std::unique_ptr _trace; const std::string& _name; }; @@ -94,10 +94,11 @@ class RunnableObject virtual void doRun() = 0; virtual void doTerminate() = 0; - void setTracing(TracingType type, std::function()> customTraceContextGetter = {}) + void setTracing(TracingType type, std::function customTraceContextGetter = {}) { _runnableObjectTracer.setTracing(type, customTraceContextGetter); } + const std::string& getName() const { return _name; }; IAlicaTrace* getTrace() const { return _runnableObjectTracer.getTrace(); }; // Helper to allow applications to generate their own trace. diff --git a/alica_engine/src/engine/RunnableObject.cpp b/alica_engine/src/engine/RunnableObject.cpp index 1bc649e11..9086511ff 100644 --- a/alica_engine/src/engine/RunnableObject.cpp +++ b/alica_engine/src/engine/RunnableObject.cpp @@ -168,7 +168,7 @@ void RunnableObject::handleException(const std::string& exceptionOriginMethod, s } // Tracing methods -void TraceRunnableObject::setTracing(TracingType type, std::function()> customTraceContextGetter) +void TraceRunnableObject::setTracing(TracingType type, std::function customTraceContextGetter) { _tracingType = type; _customTraceContextGetter = std::move(customTraceContextGetter); From 0cfd7c1db2c62765361203c682df98f411c4c7ab Mon Sep 17 00:00:00 2001 From: Dale-Koenig Date: Wed, 6 Mar 2024 15:55:01 +0900 Subject: [PATCH 3/5] Make variant public but const --- alica_engine/include/engine/IAlicaTrace.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/alica_engine/include/engine/IAlicaTrace.h b/alica_engine/include/engine/IAlicaTrace.h index 44dec187d..e49ae9834 100644 --- a/alica_engine/include/engine/IAlicaTrace.h +++ b/alica_engine/include/engine/IAlicaTrace.h @@ -35,8 +35,7 @@ class IAlicaTrace { } - private: - Variant variant; + const Variant variant; }; public: From b6fa0d4c3f6ecdf1489575f69b145330e19da90f Mon Sep 17 00:00:00 2001 From: Dale-Koenig Date: Mon, 11 Mar 2024 10:19:35 +0900 Subject: [PATCH 4/5] Remove friend declaration and extractVariant function --- alica_engine/include/engine/IAlicaTrace.h | 9 --------- alica_tests/include/alica_tests/TestTracing.h | 2 +- supplementary/alica_tracing/src/tracing/Trace.cpp | 4 ++-- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/alica_engine/include/engine/IAlicaTrace.h b/alica_engine/include/engine/IAlicaTrace.h index e49ae9834..2e73defab 100644 --- a/alica_engine/include/engine/IAlicaTrace.h +++ b/alica_engine/include/engine/IAlicaTrace.h @@ -24,8 +24,6 @@ class IAlicaTrace public: class TraceValue { - friend IAlicaTrace; - using Variant = std::variant; public: @@ -48,13 +46,6 @@ class IAlicaTrace // leave the trace in a valid but unspecified state. Calling context on a finished trace is a valid operation virtual void finish() = 0; virtual std::string context() const = 0; - -protected: - template - static decltype(auto) extractVariant(V&& trace_value) - { - return (std::forward(trace_value).variant); - } }; namespace tracing diff --git a/alica_tests/include/alica_tests/TestTracing.h b/alica_tests/include/alica_tests/TestTracing.h index 8c624f668..62518515f 100644 --- a/alica_tests/include/alica_tests/TestTracing.h +++ b/alica_tests/include/alica_tests/TestTracing.h @@ -62,7 +62,7 @@ class AlicaTestTrace : public alica::IAlicaTrace ss << v; return std::move(ss).str(); }, - extractVariant(val)); + val.variant); } }; diff --git a/supplementary/alica_tracing/src/tracing/Trace.cpp b/supplementary/alica_tracing/src/tracing/Trace.cpp index 8360e0233..f3eda18f5 100644 --- a/supplementary/alica_tracing/src/tracing/Trace.cpp +++ b/supplementary/alica_tracing/src/tracing/Trace.cpp @@ -57,7 +57,7 @@ Trace::~Trace() void Trace::setTag(std::string_view key, TraceValue value) { - _rawTrace->SetTag(prepareStringView(key), prepareRawTraceValue(extractVariant(std::move(value)))); + _rawTrace->SetTag(prepareStringView(key), prepareRawTraceValue(std::move(value).variant)); } void Trace::setTag(const std::string& key, const RawTraceValue& value) @@ -71,7 +71,7 @@ void Trace::log(const std::unordered_map& fields) RawFields raw_fields; raw_fields.reserve(fields.size()); std::transform(begin(fields), end(fields), std::back_inserter(raw_fields), - [](const auto& v) { return std::make_pair(prepareStringView(v.first), prepareRawTraceValue(extractVariant(v.second))); }); + [](const auto& v) { return std::make_pair(prepareStringView(v.first), prepareRawTraceValue(v.second.variant)); }); _rawTrace->Log(opentracing::SystemClock::now(), raw_fields); } From a299c91544ff379a8fee292e1d19d61dcf9b86cd Mon Sep 17 00:00:00 2001 From: Dale-Koenig Date: Mon, 11 Mar 2024 10:21:57 +0900 Subject: [PATCH 5/5] Nonconst --- alica_engine/include/engine/IAlicaTrace.h | 2 +- supplementary/alica_tracing/src/tracing/Trace.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/alica_engine/include/engine/IAlicaTrace.h b/alica_engine/include/engine/IAlicaTrace.h index 2e73defab..ed06f5169 100644 --- a/alica_engine/include/engine/IAlicaTrace.h +++ b/alica_engine/include/engine/IAlicaTrace.h @@ -33,7 +33,7 @@ class IAlicaTrace { } - const Variant variant; + Variant variant; }; public: diff --git a/supplementary/alica_tracing/src/tracing/Trace.cpp b/supplementary/alica_tracing/src/tracing/Trace.cpp index f3eda18f5..68b9cc927 100644 --- a/supplementary/alica_tracing/src/tracing/Trace.cpp +++ b/supplementary/alica_tracing/src/tracing/Trace.cpp @@ -57,7 +57,7 @@ Trace::~Trace() void Trace::setTag(std::string_view key, TraceValue value) { - _rawTrace->SetTag(prepareStringView(key), prepareRawTraceValue(std::move(value).variant)); + _rawTrace->SetTag(prepareStringView(key), prepareRawTraceValue(std::move(value.variant))); } void Trace::setTag(const std::string& key, const RawTraceValue& value)