jaegertracing / jaeger-client-csharp

🛑 This library is DEPRECATED!
https://jaegertracing.io/
Apache License 2.0
302 stars 67 forks source link

Read configuration over appsettings.json #133

Closed N3IS closed 5 years ago

N3IS commented 5 years ago

Requirement - what kind of business use case are you trying to solve?

I would like to create a tracer with my configurations set in the appsettings.json instead of the environment variables.

Problem - what in Jaeger blocks you from solving the requirement?

The Jaeger.Configuration.cs

Proposal - what do you suggest to solve the problem or improve the existing situation?

I solved the problem for myself by writing my necessary environment variables in the appsettings.json in a new Jaeger section and copied the Jaeger.Configuration.cs in a new class library. My new Jaeger section in appsettings.json:

  "Jaeger": {
    "JAEGER_SERVICE_NAME": "service",
    "JAEGER_AGENT_HOST": "localhost",
    "JAEGER_AGENT_PORT": "6831",
    "JAEGER_SAMPLER_TYPE": "const",
    "JAEGER_SAMPLER_PARAM": "1"
  },

To use this section I gave the "Configuration.FromEnv" method the Jaeger section as IConfigurationSection as a new parameter and wrote a new field "_configurationSection" which got the parameter as new value. After that I changed the "GetProperty" method to :

    private static string GetProperty(string name)
    {
        return _configurationSection[name];
    }

At the end I renamed all methods with "FromEnv" to "FromApp" and it works fine. But now I have to check by every update the new Configuration.cs and my own that they will still work.

This are just a few small changes in the code to use the appsettings.json as configuration file. Wouldn't it be possible to implement this feature into the Configuration File or to create two separate files "CofigurationEnv.cs" and "ConfigurationApp.cs"?

Any open questions to address

Falco20019 commented 5 years ago

Duplicate of #110. Please use the following code:

var configuration = new ConfigurationBuilder()
  .AddJsonFile("appsettings.json")
  .Build()
  .GetSection("Jaeger");
var tracer = Configuration.FromIConfiguration(_loggingFactory, configuration).GetTracer();
N3IS commented 5 years ago

Thanks for the fast answer but i have tested the new FromIConfiguration method, but it isn't really working because the FromIConfiguration calls all old FromEnv methods for the Reporter, Sampler and Codec.

https://github.com/jaegertracing/jaeger-client-csharp/blob/master/src/Jaeger/Configuration.cs#L167

And the FromEnv methods like SamplerConfiguration.FromEnv are creating there own new configuration with the environments variables which are passed to the new SamplerConfiguration.FromIConfiguration method which is only used this way.

https://github.com/jaegertracing/jaeger-client-csharp/blob/master/src/Jaeger/Configuration.cs#L340

Did I miss something or is this a bug?

Falco20019 commented 5 years ago

That's a bug :) Thanks for recognizing. I will create a PR for it.

Falco20019 commented 5 years ago

It should arrive in v0.3.1 that I hope to release beginning of next week, depending on the progress of the PRs.

Falco20019 commented 5 years ago

v0.3.1 has just been released and should be listed on NuGET in the next hour.