Annotate throttled profiles

[aleks-p]

With #3879 and #3914, distributors reject profiles when an externally controlled ingest limit is reached.

This change annotates the few profiles that are let through in this state, so that users of the profiling data can be made aware that data is being rejected. The information is stored in an extra column in the parquet table, and is applied to v1 and v2 read and write paths.

A few things are still not finished, namely:

• deduplication in the read path
• increase test coverage
• solidify naming and API contract

[simonswine]

In theory we could achieve the same thing with just using the Comments field above in L322.

I guess comment strings would be in the string table, unsure if that is a good or a bad thing.

I think if we would go for a separate type/field, I think we should make it Key,Value, because that feels more extensible to me than a []string,

[korniltsev]

if we go with this, maybe at some point we can stop exploiting build Ids for github integration

[aleks-p]

Apologies for the delay, I had to switch focus.

In theory we could achieve the same thing with just using the Comments field above in L322.
I guess comment strings would be in the string table, unsure if that is a good or a bad thing.

I considered using Comments at first, but because the strings end up in the string table there is no efficient way to read them back.

I think if we would go for a separate type/field, I think we should make it Key,Value, because that feels more extensible to me than a []string,

I agree. In parquet the data is stored in a more structured way:

phlareparquet.NewGroupField(AnnotationsColumnName, parquet.List(
			phlareparquet.Group{
				phlareparquet.NewGroupField("Body", parquet.Encoded(parquet.String(), &parquet.DeltaByteArray)),
			})),

[]string is only used in the in-memory representation currently, but a key/value will likely make merging data somewhat easier. I will give that a try.

[aleks-p]

I plan to add more tests but the critical pieces should be in place now.

[simonswine]

Nice one! I couldn't think of an areas that you forgot about. I also haven't tried this in action, but so far so good. A couple of thoughts I had inline.

[simonswine]

As we seem to except annotations from external sources, should we limit a certain prefix to internal use? That might help to trust the them slightly more

We could say everything starting with __, we wouldn't accept in the distributors.

Also potentially we might want to remove the annotations in distributors all together. Wdyt?

[aleks-p]

That is a great point.

I didn't intend for annotations to be exposed in the API but we have to do it this way because we reuse the proto messages between distributors and ingesters.

I like the idea with prefixing "trustworthy" annotations, as well as (at least temporarily) rejecting external annotations.

[simonswine]

Leave this up to you, both of them are better than not checking at all.

I guess we could always loosen the criteria.

[simonswine]

I think it is also worth documenting, how annotations are different from labels.

• They can't be used at query time to select on.
• There can be multiple annotations with the same key (with labels we don't allow duplicates).