feature: support more than 64 native attributes

[KishanPRao]

Summary

Removes the 64-entry limit on native (Crashpad) attributes.

Previously, a simple dictionary was used for storing attributes, which limits the total count to 64 entries. They are now stored in Crashpad's global AnnotationList.

The attribute count is limited by Crashpad's internal limit, kMaxNumberOfAnnotations. SetAnnotation will refuse to add keys beyond the limit, providing feedback with a warning.

Changes

• Removes use of SimpleStringDictionary, and replaces it with AnnotationList. This data is dynamically allocated.
• Native crash reports omit attributes with an empty-string value (this was allowed previously). Managed/JVM reports are unaffected, documented inaddAttribute.

ref: BT-6868

[melekr]

Nice change @KishanPRao, moving dynamic attributes off SimpleStringDictionary is the right direction and should remove the 64 entry limit.

I'm adding a few small correctness/edge-case notes below, mostly around making the new behaviour explicit and covered by tests, should be straightforward to address.

[melekr]

Crashpad still caps annotation list at kMaxNumberOfAnnotations, currently 200 on the main fork but I think we updated it a while ago to 400, so this is not strictly unbounded.

Could we update the PR wording/comments to something like “supports more than 64 dynamic attributes” or “supports dynamically allocated attributes up to Crashpad’s annotation list read limit”?

[melekr]

Crashpad still caps annotation list at kMaxNumberOfAnnotations, currently 200 on the main fork but I think we updated it a while ago to 400, so this is not strictly unbounded.

Could we update the PR wording/comments to something like “supports more than 64 dynamic attributes” or “supports dynamically allocated attributes up to Crashpad’s annotation list read limit”?

a regression test proving 65+ dynamic attributes make it into the native report/upload can be useful here. If we want a runtime guard to make the behaviour less surprising, something along these lines could work:

// Keep this aligned with Crashpad's snapshot/snapshot_constants.h kMaxNumberOfAnnotations. 
// Crashpad currently reads at most 400 annotation objects from a module.
constexpr size_t kMaxCrashpadAnnotationObjectsRead = 400;

bool WouldExceedCrashpadAnnotationReadLimit(const std::string& key) {
    return g_annotations.find(key) == g_annotations.end()
            && g_annotations.size() >= kMaxCrashpadAnnotationObjectsRead;
}

[KishanPRao]

Crashpad still caps annotation list at kMaxNumberOfAnnotations, currently 200 on the main fork but I think we updated it a while ago to 400, so this is not strictly unbounded.
Could we update the PR wording/comments to something like “supports more than 64 dynamic attributes” or “supports dynamically allocated attributes up to Crashpad’s annotation list read limit”?

a regression test proving 65+ dynamic attributes make it into the native report/upload can be useful here. If we want a runtime guard to make the behaviour less surprising, something along these lines could work:

// Keep this aligned with Crashpad's snapshot/snapshot_constants.h kMaxNumberOfAnnotations. 
// Crashpad currently reads at most 400 annotation objects from a module.
constexpr size_t kMaxCrashpadAnnotationObjectsRead = 400;

bool WouldExceedCrashpadAnnotationReadLimit(const std::string& key) {
    return g_annotations.find(key) == g_annotations.end()
            && g_annotations.size() >= kMaxCrashpadAnnotationObjectsRead;
}

Since we'd need to add an entire gtest suite, for native testing, I'd handle this separately, at a later point.