search

grafana / grafana-app-sdk

An SDK for developing apps for grafana using kubernetes-like storage and operators
Apache License 2.0
44 stars 8 forks source link

Add Generic Annotation Metadata Functions #266

Closed IfSentient closed 3 months ago

IfSentient commented 4 months ago

Add Read/Write functions for arbitrary grafana/app platform metadata encoded into annotations, for any generic type. Less efficient than generated code for a type as it requires reflection, but can be used for inspecting resources where the kind is generic or unknown at compile-time. Technically, these can do more than the codegen custom metadata as they can handle any types (not just string and time.Time). Restrictions on codegen custom metadata types are due to thema restrictions and should likely also be lifted for non-thema CUE kinds (but that requires separate work).

Also looking for feedback as to the usefulness of these functions, and naming.

IfSentient commented 4 months ago

Overall looks good.

My one concern is that annotations can only hold a limited amount of data, plus I'm not sure what would be the use-cases for attaching non-string data there.

We already store time.Time data in annotations, and it would be useful to allow other primitive types (such as int, bool, float). Small structs may also be useful for users. There's no max length on an annotation value as far as I know, but I did discover (after making this PR), that there is a limit on the max size of all annotations (keys + values) of 256kB, so there may be varying degrees of usefulness to this.

I mainly wanted a standardized way of writing/reading grafana annotations for usage when not using the codegen, so we don't see folks trying to replicate the behavior and not properly dealing with changes if/when they happen.

radiohead commented 4 months ago

Overall looks good. My one concern is that annotations can only hold a limited amount of data, plus I'm not sure what would be the use-cases for attaching non-string data there.

We already store time.Time data in annotations, and it would be useful to allow other primitive types (such as int, bool, float). Small structs may also be useful for users. There's no max length on an annotation value as far as I know, but I did discover (after making this PR), that there is a limit on the max size of all annotations (keys + values) of 256kB, so there may be varying degrees of usefulness to this.

I mainly wanted a standardized way of writing/reading grafana annotations for usage when not using the codegen, so we don't see folks trying to replicate the behavior and not properly dealing with changes if/when they happen.

Sure, I don't have strong opinion against that. I have a minor concern that instead of using spec / status we'd end up encouraging users to store data in annotations instead. I think it's fine to store some flags / timestamps there but storing slices & structs doesn't sound right to me.