Fix collection was modified error

[gamefreakru]

Cause possible this error ERROR|Jaeger.Reporters.RemoteReporter|QueueProcessor error ░ System.InvalidOperationException: Collection was modified; enumeration operation may not execute. ░ at System.Collections.Generic.List1.Enumerator.MoveNextRare() ░ at Jaeger.Reporters.Protocols.JaegerThriftSpanConverter.BuildLogs(IEnumerable1 logs) ░ at Jaeger.Reporters.Protocols.JaegerThriftSpanConverter.ConvertSpan(Span span) ░ at Jaeger.Senders.ThriftSender.AppendAsync(Span span, CancellationToken cancellationToken) ░ at Jaeger.Reporters.RemoteReporter.ProcessQueueLoop()

Which problem is this PR solving?

• Solve collection was modified exception

Short description of the changes

-

[Falco20019]

Hi @epitaphprince, thanks for your PR.

This change would highly increase object creation on Heap and also worsen the performance for all users. It would be better to ensure on the calling side that the instances are not changed. Either by calling .ToArray there or to ensure that the list is not changed through using thread-synchronization, which would be the better idea performance-wise.

I will have a look at it in more depth how Microsoft usually handles cases like this in the framework, but I assume they will also just declare it as not thread-safe.

Do you know if the enumeration was changed by your application code or if it was framework internal?

[Falco20019]

I had a look at the code right now. It seems that you are using the Log method on a Span after it finished. This is currently not expected.

Checking the code, I saw that the Span.GetLogs() method has to be fixed anyway. The current implementation is insecure because a caller could just cast it back to IList and modify it. So I will modify the code to return a new List.AsReadOnly which will also fix this bug :)

[gamefreakru]

Hi! Yes we have issue in our code, but can't repeat it locally. So we saw that in client you use IEnumerable instead ReadOnlyCollection. Please fix it :)

[Falco20019]

130 makes the logs list immutable which will also fix this problem here. I also added tests for that. Will be included in next release that I hope to release in 2 weeks. Still waiting for some reviews.

[Falco20019]

@epitaphprince Version 0.3.0 was just released and should be available through NuGET in the next hour.