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

Configure ResourceAttributes via appsettings.json #398

Closed
steverb38 opened this issue Sep 12, 2023 · 8 comments
Closed

Configure ResourceAttributes via appsettings.json #398

steverb38 opened this issue Sep 12, 2023 · 8 comments

Comments

@steverb38
Copy link

Hi,

Any suggestions on how to specifiy ResourceAttributes when configuring the sink via appsettings.json?
Specifying Endpoint and Protocol in appsettings.json works fine, but I can't get the syntax for specifying the Dictionary<string, object> for ResourceAttributes:

This is my attempt in appsettings.json - the service.name attribute doesn't end up as "myapp-test", but some default value "unknown_service:...":

   "WriteTo": [
      {
        "Name": "OpenTelemetry",
        "Args": {
          "endpoint": "http://MyOtelCollectorServer:4318/v1/logs",
          "protocol": "HttpProtobuf",
          "resourceAttributes": {
			"service.name" : "myapp-test"
		   }
		}
          }
    ],

Is this even possible?

Steve

@nblumhardt
Copy link
Member

Thanks for raising this. I don't believe that it is, at the present time. I'll move this ticket to serilog-settings-configuration, which is where the relevant code lives.

@nblumhardt nblumhardt transferred this issue from serilog/serilog-sinks-opentelemetry Sep 12, 2023
@steverb38
Copy link
Author

@nblumhardt - thanks for the reply. Good to know this wasn't me misunderstanding something! 😄

@0xced
Copy link
Member

0xced commented Sep 13, 2023

As @nblumhardt said, it's not currently possible. I did some early investigation and it seems would be pretty easy (too easy?) to make this configuration work.

"WriteTo:OpenTelemetry": {
  "Name": "OpenTelemetry",
  "Args": {
    "configure": {
      "Endpoint": "http://MyOtelCollectorServer:4318/v1/logs",
      "Protocol": "HttpProtobuf",
      "ResourceAttributes": {
        "service.name" : "myapp-test"
      },
      "BatchingOptions": {
        "BatchSizeLimit": 500
      }
    }
  }
}

Currently, it throws System.ArgumentException

Configuration resolution for Action parameter type at the path Serilog:WriteTo:OpenTelemetry:Args:configure is not implemented.

Sneak peek / starting point for a pull request:

diff --git a/src/Serilog.Settings.Configuration/Settings/Configuration/ConfigurationReader.cs b/src/Serilog.Settings.Configuration/Settings/Configuration/ConfigurationReader.cs
index 1049b85..d54a0aa 100644
--- a/src/Serilog.Settings.Configuration/Settings/Configuration/ConfigurationReader.cs
+++ b/src/Serilog.Settings.Configuration/Settings/Configuration/ConfigurationReader.cs
@@ -283,6 +283,11 @@ class ConfigurationReader : IConfigurationReader
         CallConfigurationMethods(methodCalls, FindEventEnricherConfigurationMethods(_configurationAssemblies, _resolutionContext.ReaderOptions.AllowInternalTypes, _resolutionContext.ReaderOptions.AllowInternalMethods), loggerEnrichmentConfiguration);
     }
 
+    void IConfigurationReader.ApplyBinding(object instance)
+    {
+        _section.Bind(instance, options => options.ErrorOnUnknownConfiguration = true);
+    }
+
     void ApplyEnrichment(LoggerConfiguration loggerConfiguration)
     {
         var enrichDirective = _section.GetSection("Enrich");
diff --git a/src/Serilog.Settings.Configuration/Settings/Configuration/IConfigurationReader.cs b/src/Serilog.Settings.Configuration/Settings/Configuration/IConfigurationReader.cs
index f3dd36f..82c9d2f 100644
--- a/src/Serilog.Settings.Configuration/Settings/Configuration/IConfigurationReader.cs
+++ b/src/Serilog.Settings.Configuration/Settings/Configuration/IConfigurationReader.cs
@@ -6,4 +6,5 @@ interface IConfigurationReader : ILoggerSettings
 {
     void ApplySinks(LoggerSinkConfiguration loggerSinkConfiguration);
     void ApplyEnrichment(LoggerEnrichmentConfiguration loggerEnrichmentConfiguration);
+    void ApplyBinding(object instance);
 }
diff --git a/src/Serilog.Settings.Configuration/Settings/Configuration/ObjectArgumentValue.cs b/src/Serilog.Settings.Configuration/Settings/Configuration/ObjectArgumentValue.cs
index 151ce5b..41ad346 100644
--- a/src/Serilog.Settings.Configuration/Settings/Configuration/ObjectArgumentValue.cs
+++ b/src/Serilog.Settings.Configuration/Settings/Configuration/ObjectArgumentValue.cs
@@ -39,7 +39,7 @@ class ObjectArgumentValue : IConfigurationArgumentValue
                 _ when configType == typeof(LoggerConfiguration) => new Action<LoggerConfiguration>(configReader.Configure),
                 _ when configType == typeof(LoggerSinkConfiguration) => new Action<LoggerSinkConfiguration>(configReader.ApplySinks),
                 _ when configType == typeof(LoggerEnrichmentConfiguration) => new Action<LoggerEnrichmentConfiguration>(configReader.ApplyEnrichment),
-                _ => throw new ArgumentException($"Configuration resolution for Action<{configType.Name}> parameter type at the path {_section.Path} is not implemented.")
+                _ => new Action<object>(configReader.ApplyBinding)
             };
         }

This change, as small as it looks, would require extensive test coverage.

@HHobeck
Copy link

HHobeck commented Feb 16, 2024

I having exactly the same problem. Are they any workarounds available? I'm using version 1.2.0 of Serilog.Sinks.OpenTelemetry

Why does the OpenTelemetry implementation of Serilog not using the build-in IHostingEnvironment.ApplicationName by default for the service.name?

@nblumhardt
Copy link
Member

Hi all 👋

Version 8.0.1-dev-00583 of Serilog.Settings.Configuration has just been published to NuGet with this change, and v2.0.0 of Serilog.Sinks.OpenTelemetry accepts resourceAttributes and headers via configuration.

Is anyone following this thread in a position to kick the tyres and send some feedback? Thanks!

CC @MikkelPorse and @frank-ang

@codymullins
Copy link

@nblumhardt looks good locally, pushing it to prod now 👍🏻

@nblumhardt
Copy link
Member

This is out in 8.0.1 RTM now :-)

@steverb38
Copy link
Author

Fantastic! Thanks for all your work on Serilog 👍

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

No branches or pull requests

5 participants