fix: pprof export incompatible period types #3198
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3197
The issue was that empty
ValueType
values (both period type and sample type) were used in the pprof response along the read path as they added after the profile merge. An emptyValueType
references string with index 0, assuming that the value is""
. However, in practice, we cannot guarantee that it's always true, which leads to period/sample type mismatches in rare cases.This was necessary in the past because the original
profile.Merge
verified the compatibility of profiles. Now that we have our own specialized pprof merge, this is no longer needed. The fix ensures that emptyValueType
is ignored during the merge. I believe this could be further simplified by removing the stubs entirely. Although a quick attempt revealed that this would require quite a few changes, I think it's best to implement this separately.