feat: add __unsymbolized__ label on ingest path

[marcsanmi]

This PR adds the __has_native_unsymbolized_profiles__ label on the ingest path.

Feel free to share your thought on the label naming

[kolesnikovae]

HasNativeUnsymbolizedProfiles = HasNativeUnsymbolizedProfiles || len(locations[i].Line) == 0

Absence of lines in a location does not mean it's unsymbolized: it might be a malformed profile, a bug, etc. – in fact, in most cases we want to drop such locations (it's possible that we already do so, but #4051 suggests that there's an issue).

Besides, in distributors, we normalize profiles: if a mapping has HasFunctions set, we're zeroing addresses to reduce the overhead.

https://github.com/grafana/pyroscope/blob/main/pkg/pprof/pprof.go#L414-L428

This means it might be a location without lines and without address, or a location that refers to a stub mapping. In all cases, we can't do anything about it.

We should validate and sanitize mappings and locations at normalization:

1. Ensure that locations without lines and without address are discarded.
2. Ensure that locations without lines and without a valid mapping (not a stub we create) are discarded.

---

HasFunctions is a mapping attribute. If the attribute is set, we should presume that the profile contains functions, and locations only refer to functions present in the profile. We already reset addresses for locations that refer to mappings with HasFunctions set.

Partially symbolized mappings should not have such attribute – another aspect to handle during normalization.

This is from the official pprof package:

// HasFunctions determines if all locations in this profile have
// symbolized function information.
func (p *Profile) HasFunctions() bool {
	for _, l := range p.Location {
		if l.Mapping == nil || !l.Mapping.HasFunctions {
			return false
		}
	}
	return true
}

Go pprof:

	// TODO: we set HasFunctions if all symbols from samples were symbolized (hasFuncs).
	// Decide what to do about HasInlineFrames and HasLineNumbers.
	// Also, another approach to handle the mapping entry with
	// incomplete symbolization results is to dupliace the mapping
	// entry (but with different Has* fields values) and use
	// different entries for symbolized locations and unsymbolized locations.
	if hasFuncs {
		b.pb.bool(tagMapping_HasFunctions, true)
	}

[marcsanmi]

Updated in 3aacd69.

[kolesnikovae]

I think we can name it the same way we refer to it when we discuss it:

__unsymbolized__

Changing the name later won't be this easy; we should be careful.

[marcsanmi]

Yes, I changed it after talking with Tolyan. That's why I mentioned to share the thoughts about the naming, otherwise any change after this, it'll be difficult. We've these 3 options, but I like your proposal:

• __unsymbolized__
• __has_native_unsymbolized_profiles__
• __has_native_profiles__

wdyt? @korniltsev

[korniltsev]

I like the __unsymbolized__
I did not like the __has_native_profiles__

[kolesnikovae]

Agreed, native is too ambiguous

[korniltsev]

while we don't have running github actions, lets run tests manually

I think TestIngestOTLP 🔴

[korniltsev]

or maybe merge in main branch. I think tests workflow is already running in main

[marcsanmi]

I've just updated the branch from main, but the tests are passing 😕

[korniltsev]

@marcsanmi I am sorry for the noise. Just checked and it is red on main branch on my machine 🤷 maybe the test is flaky. I need to double check later

[korniltsev]

I think mappingId==0 is a valid mapping as it should be an index into rewritten slice of mappings

[korniltsev]

I think the other case int(loc.MappingId) >= len(mappings) is also impossible to miss given our dedupSlice implementation

[korniltsev]

func TestUnsymbolized(t *testing.T) {
	profile := &googlev1.Profile{
		StringTable: []string{"", "a"},
		Function: []*googlev1.Function{
			{Id: 4, Name: 1},
		},
		Mapping: []*googlev1.Mapping{{Id: 239, HasFunctions: true}},
		Location: []*googlev1.Location{
			{Id: 5, MappingId: 239, Line: []*googlev1.Line{{FunctionId: 4, Line: 1}}},
		},
		Sample: []*googlev1.Sample{
			{LocationId: []uint64{5}, Value: []int64{1}},
		},
	}
	symbols := NewPartitionWriter(0, &Config{
		Version: FormatV3,
	})
	symbols.WriteProfileSymbols(profile)
	unsymbolized := HasUnsymbolizedProfiles(symbols.Symbols())
	assert.False(t, unsymbolized)
}

Please double check the above profile. It looks like it should not have __unsymbolized__

[korniltsev]

lgtm

[korniltsev]

I think TestIngestOTLP 🔴

It fails if run on go 1.24 if I switch back to 1.23 it does not fail. I think we should run tests on 1.24 for easier future upgrades.

[Copilot]

Pull Request Overview

This PR adds support for unsymbolized profiles by introducing a new label on the ingest path and updating several functions to use a datasets map instead of heads.

• Refactored ingest and flush functions to use a datasets map.
• Updated logging messages and error messages to reflect the new naming.
• Added tests and helper functions to evaluate unsymbolized profiles.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
pkg/experiment/ingester/segment.go	Renamed heads to datasets and updated related log/error messages.
pkg/experiment/ingester/memdb/head_test.go	Added tests for unsymbolized profile handling.
pkg/experiment/ingester/memdb/head.go	Introduced HasUnsymbolizedProfiles logic in flush implementation.
pkg/experiment/block/metadata/metadata_labels.go	Added new label constant for unsymbolized profiles.

[Copilot]

Consider adding a nil check or verifying that mappings[loc.MappingId] exists before accessing its HasFunctions field to avoid potential nil pointer dereferences.

Suggested change

	if !mappings[loc.MappingId].HasFunctions {
	mapping := mappings[loc.MappingId]
	if mapping == nil {
	continue
	}
	if !mapping.HasFunctions {