extend UsePrometheus() with optional bool parameter collectAspMetrics

[jbrekle]

true: collect ASP request URLs and timings (as before) false: don't collect anything (only what you explicitly log) Default value true to be backward compatible.

[gideonkorir]

Can we use an options class rather than a flag? E.g.

  CollectHttpMetrics = true
})

Using an options class allows us to gracefully add new options without them being breaking provided we have reasonable defaults.

[gideonkorir]

Looks like my code was cut out, I meant:

UsePrometheus(new PrometheusOptions()
{
 CollectHttpMetrics = true
});

[ahoka]

I don't think that it's future proof enough.

[gideonkorir]

@ahoka not bullet proof but u could in v2.8 add a new options and I'll just update references without having to change my code. Options are also more readable than a Boolean value.

[kodfodrasz]

Hi,

@jbrekle Thanks for the proposed PR, but I'm afraid I agree to @ahoka 's opinion about its limited extensibility. Adding more and more flags in the future may not work: they may not be clean enough when more complex config needs arise.

Regarding default operation: We have made a mistake when we enabled ASP metrics collection by default. We have realized this problem both from the discussion here, on GitHub, and also from our own experiences, especially about the Prometheus metrics explosion (see: #15). I believe we should rather break backwards compatibility now, but then provide a future proof approach, rather than trying to stick to poorly chosen defaults only for the sake of backwards compatibility.

@gideonkorir The config object pattern is versatile enough indeed, but I think we should stick closer to the ASP.Net Core practice and use more fluent, builder pattern based configuration approach.

For these reasons, and because @ahoka has come up with a PR about his approach, which fits the preferences above the best I decided that his solution shall be merged, and a new major version will be released with the notice about the breaking change in the API.

I sincerely hope that even though this PR is not accepted, you will still consider to contribute to the project in the future, should you see an opportunity.

Best Regards, @kodfodrasz

[jbrekle]

sure, whatever solves the issue is fine for me