Fix bug preventing accurate reporting of dropped attribute count

[jack-berg]

Fixes #7135

[jack-berg]

Because AttributeMap extends HashMap, a caller was accidentally bypassing this method and calling Map#put(T, V) directly, which was preventing the application of attribute limits.

We protect against this by delegating to HashMap instead of extending.

[jack-berg]

This was the problematic call which was bypassing attribute limits.

[trask]

(I suspect extending may have been intentional as a performance optimization to avoid the 4 bytes allocated to the delegate field)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.87%. Comparing base (b5daeb0) to head (115f39e).
Report is 7 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7142      +/-   ##
============================================
+ Coverage     89.85%   89.87%   +0.02%     
- Complexity     6614     6624      +10     
============================================
  Files           740      740              
  Lines         19991    20007      +16     
  Branches       1964     1968       +4     
============================================
+ Hits          17962    17982      +20     
+ Misses         1439     1438       -1     
+ Partials        590      587       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jack-berg]

I suspect extending may have been intentional as a performance optimization to avoid the 4 bytes allocated to the delegate field

The thought crossed my mind. We could keep extending, and override the implementation of all unsupported map methods with versions that throw exceptions.

[jack-berg]

We could keep extending, and override the implementation of all unsupported map methods with versions that throw exceptions.

This doesn't really work. A bunch of stuff breaks when you override the base map methods with versions that throw. You very quickly get into territory of asking "why is this even a map", and the only answer is "performance".

The choices are:

• Fix this case where we bypass the limits, but continue to extend HashMap and possibly encounter similar issues in the future. Slightly less memory allocated for basically every span.
• Delegate to HashMap instead of extending, which makes it easy to verify that no caller is bypassing the limits. Slightly more memory allocated for basically every span.

[jack-berg]

The choices are:

• Fix this case where we bypass the limits, but continue to extend HashMap and possibly encounter similar issues in the future. Slightly less memory allocated for basically every span.
• Delegate to HashMap instead of extending, which makes it easy to verify that no caller is bypassing the limits. Slightly more memory allocated for basically every span.

I pushed a commit which reverts to extending HashMap. I added some javadoc calling attention to the potential pitfalls, and adjusted the put method signature @Override the super method, making it less error prone.

Given that its an internal class and changing to a delegation model would increase memory allocation, continuing to extend HashMap and fixing all internal callers seems to be the conservative approach.