I didn't notice the requirement that Id is non null before. Given this, I wonder if we should add a constructor like UserDetails(string id), as it's a more idiomatic way to indicate a required value. Obviously, with the switch to a struct, this is not quite as nice, as you can still do new UserDetails(), but I think it's still preferable.

I added the UserDetails(string id), but still allow the setting of the Id property, as I think many users would prefer to set all properties the same way, even if the requirements vary slightly.

Related: is string.Empty a valid Id? I guess it's a valid tag value, but does it make any sense in this API, given you presumably require an ID value if they're setting other values?

To be honest, not checking for the empty string was an oversight, but I've added a check for it. I do wonder if we might come across some clients who are using the empty string for a special user id, but I think adding the check is okay, since it's easier to relax restrictions than increase them.

Also, given UserDetails is a new public API, I think we should enable nullable in UserDetails, and mark all the non-Id parameters as string? instead of string. That further hammers home the requirement

Yes, I have added this now.

Code Coverage Report 📊

✔️ Merging #2682 into master will not change line coverage
✔️ Merging #2682 into master will not change branch coverage
✔️ Merging #2682 into master will will decrease complexity by 2

View the full report for further details:

• HTML report for master | Source file
• HTML report for this PR #2682 | Source file

Datadog.Trace Breakdown ✔️

The following classes have significant coverage changes.

The following classes were added in #2682:

View the full reports for further details:

• HTML report for master | Source file
• HTML report for this PR #2682 | Source file

Benchmarks Report 🐌

Benchmarks for #2682 compared to master:

• 1 benchmarks are faster, with geometric mean 1.162
• All benchmarks have the same allocations

The following thresholds were used for comparing the benchmark speeds:

• Mann–Whitney U test with statistical test for significance of 5%
• Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details