Skip to content

Commit

Permalink
service: allow services to have defined stability levels (#6674)
Browse files Browse the repository at this point in the history
This commit adds stability levels to services. Every service, with the
exception of remotecfg, is then given a stability level of stable.
remotecfg is given a stability level of "beta."
  • Loading branch information
rfratto authored Mar 13, 2024
1 parent 3b24a9d commit 448b245
Show file tree
Hide file tree
Showing 11 changed files with 128 additions and 1 deletion.
2 changes: 1 addition & 1 deletion internal/component/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ type Registration struct {
// sure the user is not accidentally using a component that is not yet stable - users
// need to explicitly enable less-than-stable components via, for example, a command-line flag.
// If a component is not stable enough, an attempt to create it via the controller will fail.
// The default stability level is Experimental.
// This field must be set to a non-zero value.
Stability featuregate.Stability

// An example Arguments value that the registered component expects to
Expand Down
2 changes: 2 additions & 0 deletions internal/flow/flow_services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ func TestServices_Configurable(t *testing.T) {
return service.Definition{
Name: "fake",
ConfigType: ServiceOptions{},
Stability: featuregate.StabilityBeta,
}
},

Expand Down Expand Up @@ -117,6 +118,7 @@ func TestServices_Configurable_Optional(t *testing.T) {
return service.Definition{
Name: "fake",
ConfigType: ServiceOptions{},
Stability: featuregate.StabilityBeta,
}
},

Expand Down
14 changes: 14 additions & 0 deletions internal/flow/internal/controller/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/go-kit/log"
"github.com/grafana/agent/internal/featuregate"
"github.com/grafana/agent/internal/flow/internal/dag"
"github.com/grafana/agent/internal/flow/internal/worker"
"github.com/grafana/agent/internal/flow/logging/level"
Expand Down Expand Up @@ -441,6 +442,19 @@ func (l *Loader) populateServiceNodes(g *dag.Graph, serviceBlocks []*ast.BlockSt

node := g.GetByID(blockID).(*ServiceNode)

// Don't permit configuring services that have a lower stability level than
// what is currently enabled.
nodeStability := node.Service().Definition().Stability
if err := featuregate.CheckAllowed(nodeStability, l.globals.MinStability, fmt.Sprintf("block %q", blockID)); err != nil {
diags.Add(diag.Diagnostic{
Severity: diag.SeverityLevelError,
Message: err.Error(),
StartPos: ast.StartPos(block).Position(),
EndPos: ast.EndPos(block).Position(),
})
continue
}

// Blocks assigned to services are reset to nil in the previous loop.
//
// If the block is non-nil, it means that there was a duplicate block
Expand Down
90 changes: 90 additions & 0 deletions internal/flow/internal/controller/loader_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package controller_test

import (
"context"
"errors"
"os"
"strings"
Expand All @@ -11,6 +12,7 @@ import (
"github.com/grafana/agent/internal/flow/internal/controller"
"github.com/grafana/agent/internal/flow/internal/dag"
"github.com/grafana/agent/internal/flow/logging"
"github.com/grafana/agent/internal/service"
"github.com/grafana/river/ast"
"github.com/grafana/river/diag"
"github.com/grafana/river/parser"
Expand Down Expand Up @@ -316,6 +318,60 @@ func TestLoader(t *testing.T) {
})
}

func TestLoader_Services(t *testing.T) {
testFile := `
testsvc { }
`

testService := &fakeService{
DefinitionFunc: func() service.Definition {
return service.Definition{
Name: "testsvc",
ConfigType: struct {
Name string `river:"name,attr,optional"`
}{},
Stability: featuregate.StabilityBeta,
}
},
}

newLoaderOptionsWithStability := func(stability featuregate.Stability) controller.LoaderOptions {
l, _ := logging.New(os.Stderr, logging.DefaultOptions)
return controller.LoaderOptions{
ComponentGlobals: controller.ComponentGlobals{
Logger: l,
TraceProvider: noop.NewTracerProvider(),
DataPath: t.TempDir(),
MinStability: stability,
OnBlockNodeUpdate: func(cn controller.BlockNode) { /* no-op */ },
Registerer: prometheus.NewRegistry(),
NewModuleController: func(id string) controller.ModuleController {
return nil
},
},
Services: []service.Service{testService},
}
}

t.Run("Load with service at correct stability level", func(t *testing.T) {
l := controller.NewLoader(newLoaderOptionsWithStability(featuregate.StabilityBeta))
diags := applyFromContent(t, l, []byte(testFile), nil, nil)
require.NoError(t, diags.ErrorOrNil())
})

t.Run("Load with service below minimum stabilty level", func(t *testing.T) {
l := controller.NewLoader(newLoaderOptionsWithStability(featuregate.StabilityStable))
diags := applyFromContent(t, l, []byte(testFile), nil, nil)
require.ErrorContains(t, diags.ErrorOrNil(), `block "testsvc" is at stability level "beta", which is below the minimum allowed stability level "stable"`)
})

t.Run("Load with undefined minimum stability level", func(t *testing.T) {
l := controller.NewLoader(newLoaderOptionsWithStability(featuregate.StabilityUndefined))
diags := applyFromContent(t, l, []byte(testFile), nil, nil)
require.ErrorContains(t, diags.ErrorOrNil(), `stability levels must be defined: got "beta" as stability of block "testsvc" and <invalid_stability_level> as the minimum stability level`)
})
}

// TestScopeWithFailingComponent is used to ensure that the scope is filled out, even if the component
// fails to properly start.
func TestScopeWithFailingComponent(t *testing.T) {
Expand Down Expand Up @@ -473,3 +529,37 @@ func (f fakeModuleController) ClearModuleIDs() {
func (f fakeModuleController) NewCustomComponent(id string, export component.ExportFunc) (controller.CustomComponent, error) {
return nil, nil
}

type fakeService struct {
DefinitionFunc func() service.Definition // Required.
RunFunc func(ctx context.Context, host service.Host) error
UpdateFunc func(newConfig any) error
DataFunc func() any
}

func (fs *fakeService) Definition() service.Definition {
return fs.DefinitionFunc()
}

func (fs *fakeService) Run(ctx context.Context, host service.Host) error {
if fs.RunFunc != nil {
return fs.RunFunc(ctx, host)
}

<-ctx.Done()
return nil
}

func (fs *fakeService) Update(newConfig any) error {
if fs.UpdateFunc != nil {
return fs.UpdateFunc(newConfig)
}
return nil
}

func (fs *fakeService) Data() any {
if fs.DataFunc != nil {
return fs.DataFunc()
}
return nil
}
2 changes: 2 additions & 0 deletions internal/service/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

"github.com/go-kit/log"
"github.com/grafana/agent/internal/component"
"github.com/grafana/agent/internal/featuregate"
"github.com/grafana/agent/internal/flow/logging/level"
"github.com/grafana/agent/internal/service"
http_service "github.com/grafana/agent/internal/service/http"
Expand Down Expand Up @@ -162,6 +163,7 @@ func (s *Service) Definition() service.Definition {
// Cluster depends on the HTTP service to work properly.
http_service.ServiceName,
},
Stability: featuregate.StabilityStable,
}
}

Expand Down
2 changes: 2 additions & 0 deletions internal/service/http/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/go-kit/log"
"github.com/gorilla/mux"
"github.com/grafana/agent/internal/component"
"github.com/grafana/agent/internal/featuregate"
"github.com/grafana/agent/internal/flow"
"github.com/grafana/agent/internal/flow/logging/level"
"github.com/grafana/agent/internal/service"
Expand Down Expand Up @@ -128,6 +129,7 @@ func (s *Service) Definition() service.Definition {
Name: ServiceName,
ConfigType: Arguments{},
DependsOn: nil, // http has no dependencies.
Stability: featuregate.StabilityStable,
}
}

Expand Down
2 changes: 2 additions & 0 deletions internal/service/labelstore/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"time"

"github.com/go-kit/log"
"github.com/grafana/agent/internal/featuregate"
"github.com/grafana/agent/internal/flow/logging/level"
agent_service "github.com/grafana/agent/internal/service"
flow_service "github.com/grafana/agent/internal/service"
Expand Down Expand Up @@ -67,6 +68,7 @@ func (s *service) Definition() agent_service.Definition {
Name: ServiceName,
ConfigType: Arguments{},
DependsOn: nil,
Stability: featuregate.StabilityStable,
}
}

Expand Down
2 changes: 2 additions & 0 deletions internal/service/otel/otel.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"

"github.com/go-kit/log"
"github.com/grafana/agent/internal/featuregate"
"github.com/grafana/agent/internal/service"
"github.com/grafana/agent/internal/util"
)
Expand Down Expand Up @@ -50,6 +51,7 @@ func (*Service) Definition() service.Definition {
Name: ServiceName,
ConfigType: nil, // otel does not accept configuration
DependsOn: []string{},
Stability: featuregate.StabilityStable,
}
}

Expand Down
2 changes: 2 additions & 0 deletions internal/service/remotecfg/remotecfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/grafana/agent-remote-config/api/gen/proto/go/agent/v1/agentv1connect"
"github.com/grafana/agent/internal/agentseed"
"github.com/grafana/agent/internal/component/common/config"
"github.com/grafana/agent/internal/featuregate"
"github.com/grafana/agent/internal/flow/logging/level"
"github.com/grafana/agent/internal/service"
"github.com/grafana/river"
Expand Down Expand Up @@ -128,6 +129,7 @@ func (s *Service) Definition() service.Definition {
Name: ServiceName,
ConfigType: Arguments{},
DependsOn: nil, // remotecfg has no dependencies.
Stability: featuregate.StabilityBeta,
}
}

Expand Down
9 changes: 9 additions & 0 deletions internal/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"fmt"

"github.com/grafana/agent/internal/component"
"github.com/grafana/agent/internal/featuregate"
)

// Definition describes an individual Flow service. Services have unique names
Expand All @@ -35,6 +36,14 @@ type Definition struct {
// or a named service doesn't exist), it is treated as a fatal
// error and the root Flow module will exit.
DependsOn []string

// Stability is the overall stability level of the service. This is used to
// make sure the user is not accidentally configuring a service that is not
// yet stable - users need to explicitly enable less-than-stable services
// via, for example, a command-line flag. If a service is not stable enough,
// an attempt to configure it via the controller will fail.
// This field must be set to a non-zero value.
Stability featuregate.Stability
}

// Host is a controller for services and Flow components.
Expand Down
2 changes: 2 additions & 0 deletions internal/service/ui/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"path"

"github.com/gorilla/mux"
"github.com/grafana/agent/internal/featuregate"
"github.com/grafana/agent/internal/service"
http_service "github.com/grafana/agent/internal/service/http"
"github.com/grafana/agent/internal/web/api"
Expand Down Expand Up @@ -46,6 +47,7 @@ func (s *Service) Definition() service.Definition {
Name: ServiceName,
ConfigType: nil, // ui does not accept configuration
DependsOn: []string{http_service.ServiceName},
Stability: featuregate.StabilityStable,
}
}

Expand Down

0 comments on commit 448b245

Please sign in to comment.