Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[question] how to copy finalized config. #171

Open
kristjanvalur opened this issue Nov 25, 2024 · 5 comments
Open

[question] how to copy finalized config. #171

kristjanvalur opened this issue Nov 25, 2024 · 5 comments

Comments

@kristjanvalur
Copy link
Contributor

I'm trying to embed the tracer in a plugin. I need to perform the config finalization in a class method. However, it appears that there is no public default constructor or a copy constructor for FinalizedTracerConfig.

i.e. I don't seem to be able to do the following:

class Foo {
public:
  void Init()
  {
    MaybeConfig = finalize_config(TracerConfig()):
    if (!MaybeConfig.if_error())
      Config = *MaybeConfig;
  }
private:
  FinalizedTracerConfig Config;
};

Compiling this won't even generate a constructor:
'Foo::Foo(void)': function was implicitly deleted because a base class invokes a deleted or inaccessible function 'datadog::tracing::FinalizedTracerConfig::FinalizedTracerConfig(void)'

As a plugin I don't have the luxury of specifying an auto variable in my main(), and a member cannot be initialized in the class construtor.
Same goes for a Tracer object. I wish to initialize a global config and a Tracer object at application startup, and then be able to refer to them as needed.

Is there some pattern I'm not seeing in how to initialized a tracing context and store it away somewhere?

@dmehala
Copy link
Collaborator

dmehala commented Nov 26, 2024

Hey @kristjanvalur

Thank you for your question.

The behaviour you're encountering is intentional. The FinalizedTracerConfig class is designed as a token that can only be created using finalize_config and must be directly consumed by the Tracer constructor. This approach ensures that a Tracer instance is always initialized with a valid configuration, providing strong guarantees about its correctness.

Suggested Solution

Based on your use case, it might be more practical to store the TracerConfig itself rather than the FinalizedTracerConfig. Here's why: while TracerConfig is mutable and can be reused across different contexts, FinalizedTracerConfig is specifically tied to the process of creating a Tracer and cannot be copied or default-constructed.

I wish to initialize a global config and a Tracer object at application startup, and then be able to refer to them as needed

If I understand correctly, you want to initialize a global configuration and a Tracer at application startup, and then use them throughout your plugin. For this purpose a singleton pattern can work well.

Here's an example to illustrate:

datadog::tracing::Tracer& tracer() {
   static datadog::tracing::Tracer tracer(*finalize_config(TracerConfig());
   return tracer;
};

void main() {
   ...
   auto span = tracer().create_span();
   ...
}

If this does not meet your requirements, could you provide a simplified example of the implementation you're aiming for? That would help us better understand your needs and explore alternative solutions.

@kristjanvalur
Copy link
Contributor Author

kristjanvalur commented Nov 26, 2024

Thanks for your reply.
I had come to the same conclusion myself, that it was intentionally made so that the FinalizedTracerConfig was not assignable.
I have arrived at a similar solution, using an unique pointer (this is in the context of UnrealEngine):

dd::TracerConfig RawConfig;
// customize the config here...
const auto Maybe_Config = dd::finalize_config(RawConfig);
if (auto* Error = Maybe_Config.if_error())
{
	UE_LOG(LogTemp, Error, TEXT("Failed to finalize config: %s"), Error->message.c_str());
	return;
}
Tracer = MakeUnique<dd::Tracer>(*Maybe_Config);

where Tracer is a FUniquePtr<dd::Tracer> instance. Years of indoctrination have made me wary of static instances :)

Thanks!

@kristjanvalur
Copy link
Contributor Author

kristjanvalur commented Nov 26, 2024

However, next issue:
In the context of an engine, a Tracer may not have been successfully created. Application code which wants to use the Span class however can not be conditional, based on the desing of the Span class.
Might it not be useful to provide a NullTracer instance in such a case, returning some NullSpan object?

Then an application could be written something like:

dd::Tracer &GetSpan();
dd::Span my_span = GetTracer().create_span();
dd::Span child_span = my_span.create_span();

i.e. logic elsewhere need not worry about the successful creation of a valid Tracer, GetTracer() could return a valid tracer or a null tracer depdending on the situation.

Or, if there is an alternative way to safely create a neutered Tracer object, in case configuration is broken, that would be good to know.

@kristjanvalur
Copy link
Contributor Author

kristjanvalur commented Nov 27, 2024

I think the documentation could be improved by showing a general framework for working with asynchronous / callback-based apis.

I found some code in the httpserver example showing a separate Span stack, will work with that.

Update: Will end up with my own graph of Span nodes connected with shared pointers.

@kristjanvalur
Copy link
Contributor Author

Ok, I suspect that the library really does not really allow for creating a non-functional Tracer in case the config cannot be finalized.
I guess the FinalizedConfig can have its report_traces set to false... But generating a FinalizedConfig might still fail because of weird environment variables.
In my case, I want to be resilient in case of misconfigured environment variables or other things. There seems to be no way to override the examination of the environment for some of the settings. Ignore the environment variables, for example and create a Tracer object with pure default settings and then disable tracing....

We have a situation where the same executable may be deployed in different ways but the presence of some other information may decide if Tracing is enabled at all, and then, it should not be overridable by the user by setting the environment in a certain way. In this case, using a NullTracer instance would be very useful.

As it is, I will need to use some wrapping code where creating Spans and dynamically allocate them if i have a Tracer at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants