Avoid usage of ConcurrentDictionary

[dotnet-architecture]

Avoid usage of ConcurrentDictionary

[ycrumeyrolle]

In addition, you may be able to use the result of the Task.WhenAll instead of the task.Value.Result in the for each Health Service loop.

[bradwilson]

It is not necessary to be thread safe in this class if the Health Service do not add the results in different threads.

It became necessary when parallelizing the checks (the ConcurrentDictionary is used to register the results).

you may be able to use the result of the Task.WhenAll

If you'll notice, the loop needed to preserve both the key and the task, so you can't use the results of the Task.WhenAll, because it's waiting on the task inside the projection, but my loop needs both the key and the task.

[ycrumeyrolle]

The CompositeHealthCheckResult.Add () is used within a sequential for each.
The parallel flow is execute by the Task.WhenAll just before, which is awaited.

The key used in the loop is the name of the check. The HealthCheckResult does not have this property, but it could be added.

[bradwilson]

I think you missed this: https://github.com/aspnet/HealthChecks/blob/93360725a04eb2270a537e41bb0b67094e39e997/src/Microsoft.Extensions.HealthChecks/Internal/UrlChecker.cs#L47

[ycrumeyrolle]

I totally missed it.
I am not sure that current implementation of http check is mature.
Http check will probably require authentication specific for each endpoint, and may be a custom validation extension point, because some endpoint returns 200 even in case of errors, or some endpoint returns 500 or 400 even if not in error.
Why allowing multicheck within this check and not in the others? If if is for a simpler registration, this can be done with a builder provider multiple checkers instead of a single multichecker.

When I look at the UrlChecker implementation, is see a lot of similitude to the HealthCheckService. I though that the check flow were managed by the service, but it is also managed by the checkers. You are what I mean?

[bradwilson]

The role of multi-checks within UrlChecker may change depending on whether we choose to support grouping (see #16). Generalized grouping would alleviate the need to UrlCheck vs. UrlChecks. /cc @glennc