Fixes

[mjwills-k]

• URL encode identity to ensure that characters like & are treated correctly
• Switch Dictionary to ConcurrentDictionary for cache so that it is thread-safe
• Better handle DST transitions
• Fix race conditions with AnalyticsDataThreads

I have not done exhaustive testing.

[matthewelwell]

Hi @mjwills-k, overall this PR looks great - thanks very much for submitting! I've added a few comments, but nothing too major.

[matthewelwell]

@mjwills-k could you also take a look at the failing check on the formatting here?

[mjwills-k]

@matthewelwell Let me know if that addresses your concerns.

[matthewelwell]

Thanks for dealing with the feedback @mjwills-k , this is great.

The one outstanding question I'm be interested to get your opinion on is whether you would expect this to be a breaking change, and hence a major version?

You could argue that the change to the identifier encoding is a bug fix, but you could equally argue that it changes the behaviour for certain identities. As someone that has been affected by this issue, I'm interested to know your opinion on the matter.

[mjwills-k]

I could argue it either way. I suppose a major version change at least highlights that the customer might be impacted. So likely warranted?

The changes to AnalyticsProcessor will also make calls to TrackFeature slower than before (I'd argue more reliable - but undeniably slower). So that might mean the increase in major version number a good idea.

Side issue - I think there might be a way to make AnalyticsProcessor be better under load with something like https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim?view=net-9.0 - but that is harder to get correct so I've decided not to do it for now. My blunt approach is at least easy to read and reason about as is.

[matthewelwell]

Thanks @mjwills-k I've released this as v6.0.0 here. You can follow the release pipeline here.