Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syntax: require all capsule values to implement marker interface #159

Open
rfratto opened this issue Nov 22, 2022 · 1 comment
Open

syntax: require all capsule values to implement marker interface #159

rfratto opened this issue Nov 22, 2022 · 1 comment
Labels
needs-attention proposal A proposal for new functionality. type/syntax Alloy configuration syntax issues

Comments

@rfratto
Copy link
Member

rfratto commented Nov 22, 2022

Today, a Go value maps to a Alloy syntax capsule when any of the following are true:

  1. The Go type implements the capsule marker interface
  2. It is a Go interface type but not interface{}
  3. It is a Go map which is not keyed by string
  4. It does not map to a Alloy null, number, string, bool, object, or function.

These rules were intended to make capsules feel "magical," have historically caused bugs (#2361, grafana/agent#2362) and are generally confusing to remember (I even had to look up the rules again just when writing this). The biggest issue with capsules is that capsule information can get lost if a type is casted to an interface{}, which can happen in many situations, including storing a capsule value in a *vm.Scope as a variable.

About a month ago I suggested changing these rules, and this issue is a more formal proposal of that suggestion.

Proposal

I propose that:

  1. Go types only map to an Alloy capsule if they implement the capsule marker interface
  2. We introduce a high-level API to encapsulate any Go value
  3. Go types which do not map to any Alloy type now cause an encoding error (rather than defaulting to a capsule type)

This proposal only affects developers building components on top of Alloy. Users of Alloy are not impacted.

The new API looks like this:

package syntax

// Capsule wraps around a value and marks it as an Alloy capsule, 
// allowing arbitrary Go types to be passed around in Alloy. 
//
// A manual capsule type can be created by implementing the 
// non-pointer receiver AlloyCapsule method on a type.    
type Capsule[T any] struct {
  val T 
} 

// Encapsulate creates a new Capsule[T] for a value.
func Encapsulate[T any](v T) Capsule[T] {
  return Capsule[T]{val: v} 
} 

// NewCapsule creates a new *Capsule[T] for a value. This is 
// useful when capsules values are optional. 
func NewCapsule[T any](v T) *Capsule[T] {
  return &Capsule[T]{val: v} 
}

// AlloyCapsule marks Capsule[T] as a capsule type. 
func (c Capsule[T]) AlloyCapsule() {} 

// Get returns the underlying value for the Capsule. If 
// the Capsule is nil, Get returns the zero value for the 
// underlying type. 
func (c *Capsule[T]) Get() T {
  if c == nil {
    var zero T 
    return zero 
  }
  return c.val 
}

Example usage

Using the new proposed API above, the exports for prometheus.remote_write change to:

package remotewrite 

... 

type Exports struct {
  Receiver syntax.Capsule[storage.Appendable] `alloy:"receiver,attr"`
}

The arguments type for types which accept capsules would also change:

package scrape // prometheus.scrape 

... 

type Arguments struct {
  ForwardTo []syntax.Capsule[storage.Appendable] `alloy:"forward_to,attr"`

  ...
}

Discussion

This proposal does come at a cost: developing against Alloy becomes slightly more tedious for capsule values, as it removes the magic introduced in the initial implementation.

However, the new rules are much easier to remember, and the new API should offload most of the tedium introduced by the new rules. This proposal even potentially makes Alloy easier to understand for developers reading the code, as it now must be explicit when something is being encapsulated.

@rfratto rfratto changed the title River: require capsule values to implement marker interface River: require all capsule values to implement marker interface Nov 23, 2022
rfratto referenced this issue in rfratto/agent Mar 6, 2023
This makes two changes to fix the loss of the capsule type when a
capsule value is converted to interface{}:

1. Structs with no River tags are no longer considered objects, and are
   now considered capsules.

2. Capsules can be converted if the Go types can be converted (such as
   assigning to an interface).

Fixes grafana#3171.
Related to #2547.
rfratto referenced this issue in rfratto/agent Mar 7, 2023
This makes two changes to fix the loss of the capsule type when a
capsule value is converted to interface{}:

1. Structs with no River tags are no longer considered objects, and are
   now considered capsules.

2. Capsules can be converted if the Go types can be converted (such as
   assigning to an interface).

Fixes grafana#3171.
Related to #2547.
rfratto referenced this issue in grafana/agent Mar 7, 2023
This makes two changes to fix the loss of the capsule type when a
capsule value is converted to interface{}:

1. Structs with no River tags are no longer considered objects, and are
   now considered capsules.

2. Capsules can be converted if the Go types can be converted (such as
   assigning to an interface).

Fixes #3171.
Related to #2547.
rfratto referenced this issue in grafana/agent Mar 7, 2023
This makes two changes to fix the loss of the capsule type when a
capsule value is converted to interface{}:

1. Structs with no River tags are no longer considered objects, and are
   now considered capsules.

2. Capsules can be converted if the Go types can be converted (such as
   assigning to an interface).

Fixes #3171.
Related to #2547.
rfratto referenced this issue in grafana/agent Mar 9, 2023
* river: fix capsule type loss when converting capsules to any

This makes two changes to fix the loss of the capsule type when a
capsule value is converted to interface{}:

1. Structs with no River tags are no longer considered objects, and are
   now considered capsules.

2. Capsules can be converted if the Go types can be converted (such as
   assigning to an interface).

Fixes #3171.
Related to #2547.

* add missing changelog entry for #3204

Co-authored-by: erikbaranowski <[email protected]>
@rfratto rfratto transferred this issue from grafana/agent Aug 25, 2023
@rfratto rfratto transferred this issue from grafana/river Apr 9, 2024
@rfratto rfratto changed the title River: require all capsule values to implement marker interface syntax: require all capsule values to implement marker interface Apr 9, 2024
@rfratto rfratto added the type/syntax Alloy configuration syntax issues label Apr 9, 2024
Copy link
Contributor

This issue has not had any activity in the past 30 days, so the needs-attention label has been added to it.
If the opened issue is a bug, check to see if a newer release fixed your issue. If it is no longer relevant, please feel free to close this issue.
The needs-attention label signals to maintainers that something has fallen through the cracks. No action is needed by you; your issue will be kept open and you do not have to respond to this comment. The label will be removed the next time this job runs if there is new activity.
Thank you for your contributions!

@rfratto rfratto added the proposal A proposal for new functionality. label Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-attention proposal A proposal for new functionality. type/syntax Alloy configuration syntax issues
Projects
Status: Incoming
Development

No branches or pull requests

1 participant