-
Notifications
You must be signed in to change notification settings - Fork 201
Implement EPP Plugins by datalayer objects #1901
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
Changes from all commits
3af0260
ec27b21
7f39815
4cb4e25
c05299a
b0b61c1
b986abc
9b72e7e
2f704b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| /* | ||
| Copyright 2025 The Kubernetes Authors. | ||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package datalayer | ||
|
|
||
| // Config defines the configuration of EPP data layer, as the set of DataSources and | ||
| // Extractors defined on them. | ||
| type Config struct { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this struct a prep for next PR?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct! |
||
| Sources []DataSourceConfig // the data sources configured in the data layer | ||
| } | ||
|
|
||
| // DataSourceConfig defines the configuration of a specific DataSource | ||
| type DataSourceConfig struct { | ||
| Plugin DataSource // the data source plugin instance | ||
| Extractors []Extractor // extractors defined for the data source | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,12 +22,13 @@ import ( | |
| "fmt" | ||
| "reflect" | ||
| "sync" | ||
|
|
||
| "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/plugins" | ||
| ) | ||
|
|
||
| // DataSource provides raw data to registered Extractors. | ||
| type DataSource interface { | ||
| // Name of this data source. | ||
| Name() string | ||
| plugins.Plugin | ||
| // Extractors returns a list of registered Extractor names. | ||
| Extractors() []string | ||
| // AddExtractor adds an extractor to the data source. Multiple | ||
|
|
@@ -45,7 +46,7 @@ type DataSource interface { | |
|
|
||
| // Extractor transforms raw data into structured attributes. | ||
| type Extractor interface { | ||
| Name() string | ||
| plugins.Plugin | ||
| // ExpectedType defines the type expected by the extractor. | ||
| ExpectedInputType() reflect.Type | ||
| // Extract transforms the raw data source output into a concrete structured | ||
|
|
@@ -65,22 +66,12 @@ func (dsr *DataSourceRegistry) Register(src DataSource) error { | |
| if src == nil { | ||
| return errors.New("unable to register a nil data source") | ||
| } | ||
| if _, loaded := dsr.sources.LoadOrStore(src.Name(), src); loaded { | ||
| return fmt.Errorf("unable to register duplicate data source: %s", src.Name()) | ||
| if _, loaded := dsr.sources.LoadOrStore(src.TypedName().Name, src); loaded { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bit confusing. maybe we can use TypedName.String() as the key of the map?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was originally using Type as the key, went back and forth on this given @shmuelk feedback on usage for configuration file. You can see some of the discussion context in resolved conversations. The plugin references in the configuration file use the plugin name as the reference key, hence the same is applied here. |
||
| return fmt.Errorf("unable to register duplicate data source: %s", src.TypedName().String()) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // GetNamedSource fetches a source by name. | ||
| func (dsr *DataSourceRegistry) GetNamedSource(name string) (DataSource, bool) { | ||
| if val, ok := dsr.sources.Load(name); ok { | ||
| if ds, ok := val.(DataSource); ok { | ||
| return ds, true | ||
| } | ||
| } | ||
| return nil, false | ||
| } | ||
|
|
||
| // GetSources returns all registered sources. | ||
| func (dsr *DataSourceRegistry) GetSources() []DataSource { | ||
| var result []DataSource | ||
|
|
@@ -100,21 +91,6 @@ func RegisterSource(src DataSource) error { | |
| return defaultDataSources.Register(src) | ||
| } | ||
|
|
||
| // GetNamedSource returns a typed data source from the default registry. | ||
| func GetNamedSource[T DataSource](name string) (T, bool) { | ||
| v, ok := defaultDataSources.GetNamedSource(name) | ||
| if !ok { | ||
| var zero T | ||
| return zero, false | ||
| } | ||
| src, ok := v.(T) | ||
| if !ok { | ||
| var zero T | ||
| return zero, false | ||
| } | ||
| return src, true | ||
| } | ||
|
|
||
| // GetSources returns the list of data sources registered in the default registry. | ||
| func GetSources() []DataSource { | ||
| return defaultDataSources.GetSources() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,20 +22,26 @@ import ( | |
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
|
|
||
| "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/plugins" | ||
| ) | ||
|
|
||
| const ( | ||
| testType = "test-ds-type" | ||
| ) | ||
|
|
||
| type mockDataSource struct { | ||
| name string | ||
| tn plugins.TypedName | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typedName? |
||
| } | ||
|
|
||
| func (m *mockDataSource) Name() string { return m.name } | ||
| func (m *mockDataSource) TypedName() plugins.TypedName { return m.tn } | ||
| func (m *mockDataSource) Extractors() []string { return []string{} } | ||
| func (m *mockDataSource) AddExtractor(_ Extractor) error { return nil } | ||
| func (m *mockDataSource) Collect(_ context.Context, _ Endpoint) error { return nil } | ||
|
|
||
| func TestRegisterAndGetSource(t *testing.T) { | ||
| reg := DataSourceRegistry{} | ||
| ds := &mockDataSource{name: "test"} | ||
| ds := &mockDataSource{tn: plugins.TypedName{Type: testType, Name: testType}} | ||
|
|
||
| err := reg.Register(ds) | ||
| assert.NoError(t, err, "expected no error on first registration") | ||
|
|
@@ -47,35 +53,25 @@ func TestRegisterAndGetSource(t *testing.T) { | |
| err = reg.Register(nil) | ||
| assert.Error(t, err, "expected error on nil") | ||
|
|
||
| // Get by name | ||
| got, found := reg.GetNamedSource("test") | ||
| assert.True(t, found, "expected to find registered data source") | ||
| assert.Equal(t, "test", got.Name()) | ||
|
|
||
| // Get all sources | ||
| all := reg.GetSources() | ||
| assert.Len(t, all, 1) | ||
| assert.Equal(t, "test", all[0].Name()) | ||
| assert.Equal(t, testType, all[0].TypedName().Type) | ||
|
|
||
| // Default registry | ||
| err = RegisterSource(ds) | ||
| assert.NoError(t, err, "expected no error on registration") | ||
|
|
||
| // Get by name | ||
| got, found = GetNamedSource[*mockDataSource]("test") | ||
| assert.True(t, found, "expected to find registered data source") | ||
| assert.Equal(t, "test", got.Name()) | ||
|
|
||
| // Get all sources | ||
| all = GetSources() | ||
| assert.Len(t, all, 1) | ||
| assert.Equal(t, "test", all[0].Name()) | ||
| assert.Equal(t, testType, all[0].TypedName().Type) | ||
| } | ||
|
|
||
| func TestGetNamedSourceWhenNotFound(t *testing.T) { | ||
| func TestGetSourceWhenNoneAreRegistered(t *testing.T) { | ||
| reg := DataSourceRegistry{} | ||
| _, found := reg.GetNamedSource("missing") | ||
| assert.False(t, found, "expected source to be missing") | ||
| found := reg.GetSources() | ||
| assert.Empty(t, found, "expected no sources to be returned") | ||
| } | ||
|
|
||
| func TestValidateExtractorType(t *testing.T) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -25,15 +25,17 @@ import ( | |||||
| "sync" | ||||||
|
|
||||||
| "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/datalayer" | ||||||
| "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/plugins" | ||||||
| ) | ||||||
|
|
||||||
| const ( | ||||||
| DataSourceName = "metrics-data-source" | ||||||
| DataSourceType = "metrics-data-source" | ||||||
| ) | ||||||
|
|
||||||
| // DataSource is a Model Server Protocol (MSP) compliant metrics data source, | ||||||
| // returning Prometheus formatted metrics for an endpoint. | ||||||
| type DataSource struct { | ||||||
| tn plugins.TypedName | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: consistency with other plugins?
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. current plugins use both |
||||||
| metricsScheme string // scheme to use in metrics URL | ||||||
| metricsPath string // path to use in metrics URL | ||||||
|
|
||||||
|
|
@@ -42,10 +44,9 @@ type DataSource struct { | |||||
| } | ||||||
|
|
||||||
| // NewDataSource returns a new MSP compliant metrics data source, configured with | ||||||
| // the provided client factory. If ClientFactory is nil, a default factory is used. | ||||||
| // The Scheme, port and path are command line options. It should be noted that | ||||||
| // a port value of zero is set if the command line is unspecified. | ||||||
| func NewDataSource(metricsScheme string, metricsPath string, skipCertVerification bool, cl Client) *DataSource { | ||||||
| // the provided client configuration. | ||||||
| // The Scheme, path and certificate validation setting are command line options. | ||||||
| func NewDataSource(metricsScheme string, metricsPath string, skipCertVerification bool) *DataSource { | ||||||
| if metricsScheme == "https" { | ||||||
| httpsTransport := baseTransport.Clone() | ||||||
| httpsTransport.TLSClientConfig = &tls.Config{ | ||||||
|
|
@@ -54,33 +55,33 @@ func NewDataSource(metricsScheme string, metricsPath string, skipCertVerificatio | |||||
| defaultClient.Transport = httpsTransport | ||||||
| } | ||||||
|
|
||||||
| if cl == nil { | ||||||
| cl = defaultClient | ||||||
| } | ||||||
|
|
||||||
| dataSrc := &DataSource{ | ||||||
| tn: plugins.TypedName{ | ||||||
| Type: DataSourceType, | ||||||
| Name: DataSourceType, | ||||||
| }, | ||||||
| metricsScheme: metricsScheme, | ||||||
| metricsPath: metricsPath, | ||||||
| client: cl, | ||||||
| client: defaultClient, | ||||||
| } | ||||||
| return dataSrc | ||||||
| } | ||||||
|
|
||||||
| // Name returns the metrics data source name. | ||||||
| func (dataSrc *DataSource) Name() string { | ||||||
| return DataSourceName | ||||||
| // TypedName returns the metrics data source type and name. | ||||||
| func (dataSrc *DataSource) TypedName() plugins.TypedName { | ||||||
| return dataSrc.tn | ||||||
| } | ||||||
|
|
||||||
| // Extractors returns a list of registered Extractor names. | ||||||
| func (dataSrc *DataSource) Extractors() []string { | ||||||
| names := []string{} | ||||||
| extractors := []string{} | ||||||
| dataSrc.extractors.Range(func(_, val any) bool { | ||||||
| if ex, ok := val.(datalayer.Extractor); ok { | ||||||
| names = append(names, ex.Name()) | ||||||
| extractors = append(extractors, ex.TypedName().String()) | ||||||
| } | ||||||
| return true // continue iteration | ||||||
| }) | ||||||
| return names | ||||||
| return extractors | ||||||
| } | ||||||
|
|
||||||
| // AddExtractor adds an extractor to the data source, validating it can process | ||||||
|
|
@@ -89,8 +90,8 @@ func (dataSrc *DataSource) AddExtractor(extractor datalayer.Extractor) error { | |||||
| if err := datalayer.ValidateExtractorType(PrometheusMetricType, extractor.ExpectedInputType()); err != nil { | ||||||
| return err | ||||||
| } | ||||||
| if _, loaded := dataSrc.extractors.LoadOrStore(extractor.Name(), extractor); loaded { | ||||||
| return fmt.Errorf("attempt to add extractor with duplicate name %s to %s", extractor.Name(), dataSrc.Name()) | ||||||
| if _, loaded := dataSrc.extractors.LoadOrStore(extractor.TypedName().Name, extractor); loaded { | ||||||
| return fmt.Errorf("attempt to add duplicate extractor %s to %s", extractor.TypedName(), dataSrc.TypedName()) | ||||||
| } | ||||||
| return nil | ||||||
| } | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.