search

DataDog / serilog-sinks-datadog-logs

Serilog Sink that sends log events to Datadog https://www.datadoghq.com/
Apache License 2.0
62 stars 42 forks source link

Allow batch size, period and queue limit to be set #41

Closed benfoster closed 4 years ago

benfoster commented 4 years ago

This PR extends the sink to allow the batch size, period and queue limit to be set. Previously these were hardcoded values which we found unsuitable for a number of applications generating a large number of log entries.

I'm still not 100% sure how configuration (e.g. appsettings.json) is auto-magically bound to the sink so have just followed the existing conventions.

Note that I had to use two constructors since the periodic batching only allows a queue limit to either be set or not (resulting in an unbound queue).

There's a couple of unreferenced JSON related fields which I look like they could be removed but again I'm unaware of they're being used for any dynamic binding.

ogaca-dd commented 4 years ago

Hi @benfoster ,

Thank you for submitting this pull request. I will review it at the beginning of next week.

ogaca-dd commented 4 years ago

I'm still not 100% sure how configuration (e.g. appsettings.json) is auto-magically bound to the sink so have just followed the existing conventions.

I have just checked it and it looks good.

benfoster commented 4 years ago

@ogaca-dd thanks for the review. I've addressed the comments on the PR.

I've reinstated the minimum log level with the slight change that I've renamed the property as I'm not sure this was actually working as intended before as we found setting restrictedToMinimumLevel in appsettings.json had no effect (most likely due to the property name binding).

IrisClasson commented 4 years ago

Are there rate limits that we have to keep in mind when changing the periodic timer to a lower value, or if we were to use an IEventLogSink instead of the periodic batching? The API has many restrictions/limits but it’s unclear if they apply to sending logs entries (https://docs.datadoghq.com/api/ ). Requests with rate limits return rate limit information in the header. The sink uses v1 of the API

ben-foster-cko commented 4 years ago

@ogaca-dd you're most welcome. Any ideas when this can be merged and released?

ogaca-dd commented 4 years ago

@IrisClasson: As far as I know, there is no rate limits for sending logs. Sending logs are not free of charge.

ogaca-dd commented 4 years ago

@ben-foster-cko : Serilog.Sinks.Datadog.Logs 0.3.2 includes this PR.