Skip to content
New issue

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

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

Already on GitHub? # to your account

limit inline options to endpoint, protocol, and headers #68

Merged
merged 4 commits into from
May 2, 2023

Conversation

loomis
Copy link
Contributor

@loomis loomis commented May 1, 2023

Limit the inline options to endpoint, protocol, and headers. If other options are needed, then the configuration with the options object should be used.

Update the example program to use the options object and correct collector configuration.

Fixes #61.

@loomis loomis self-assigned this May 1, 2023
@loomis loomis marked this pull request as ready for review May 2, 2023 12:34
@loomis loomis merged commit d6913b7 into dev May 2, 2023
@loomis loomis deleted the limit-inline-options branch May 2, 2023 12:39
@nblumhardt
Copy link
Member

Looking at this again, how would you feel about leaving headers out, too? Keep it super, super simple? The Serilog.Settings.Configuration feature set won't support dictionary args as things stand, anyway, I think.

@loomis
Copy link
Contributor Author

loomis commented May 3, 2023

Most real use cases will be using an authenticated endpoint, so they won't be able to use the inline configuration. That's OK with me though. Realistically, I think most people should be using the options interface instead.

@nblumhardt
Copy link
Member

👍 I think real-world use will also almost always specify resource attributes; I'm leaning in the same direction (towards the options interface) too.

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

Successfully merging this pull request may close these issues.

Move less-frequently-used configuration options out of the inline WriteTo.OpenTelemetry() overload
3 participants