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

Add template provider #8504

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .chloggen/template-provider.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: confmap

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add template provider

# One or more tracking issues or pull requests related to the change
issues: [8504]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
This provider reads and validate template files, but the templates cannot be expanded without an associated template converter.

# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [api]
109 changes: 109 additions & 0 deletions confmap/provider/templateprovider/provider.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package templateprovider // import "go.opentelemetry.io/collector/confmap/provider/templateprovider"

import (
"context"
"fmt"
"html/template"
"os"
"path/filepath"
"strings"

"go.opentelemetry.io/collector/confmap"
"go.opentelemetry.io/collector/confmap/provider/internal"
)

const (
schemeName = "template"
templateKey = "template"
typeKey = "type"

// The template provider will always return the following structure:
// {
// "templates": {
// "the_type_of_template": "the_template"
// },
// }
// This allows multiple templates to be aggreated under the same global key.
allTemplatesKey = "templates"
)

type provider struct{}

// New returns a new confmap.Provider that reads the template from a file.
//
// This Provider supports "file" scheme, and can be called with a "uri" that follows:
Copy link
Contributor

@evan-bradley evan-bradley Sep 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way to take advantage of the other providers to handle obtaining the templates? Since templates feel more like another type of config than a config "source" (file, http, s3, etc.), it would be unfortunate if templates could either only be obtained from select sources or that getting them from additional sources would require updates to the provider.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look at this @evan-bradley. I worked out an implementation based only on a converter. (See #8507)

I actually like this a lot better.

  • It has the advantage you mentioned, of allowing any provider to provide templates.
  • It removes the awkward coupling between a provider and converter.
  • It allows templates to appear directly in a config file (though I think typically they would not be used this way.) Still, it is nice to be able to reason about them in a more natural way.

//
// file-uri = "template:" local-path
// local-path = [ drive-letter ] file-path
// drive-letter = ALPHA ":"
//
// The "file-path" can be relative or absolute, and it can be any OS supported format.
//
// Examples:
// `template:path/to/template` - relative path (unix, windows)
// `template:/path/to/template` - absolute path (unix, windows)
// `template:c:/path/to/template` - absolute path including drive-letter (windows)
// `template:c:\path\to\template` - absolute path including drive-letter (windows)
func New() confmap.Provider {
return &provider{}
}

func (fmp *provider) Retrieve(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) {
if !strings.HasPrefix(uri, schemeName+":") {
return nil, fmt.Errorf("%q uri is not supported by %q provider", uri, schemeName)
}

// Clean the path before using it.
content, err := os.ReadFile(filepath.Clean(uri[len(schemeName)+1:]))
if err != nil {
return nil, fmt.Errorf("unable to read the file %v: %w", uri, err)
}

retrieved, err := internal.NewRetrievedFromYAML(content)
if err != nil {
return nil, fmt.Errorf("read template %v: %w", uri, err)
}

templateConf, err := retrieved.AsConf()
if err != nil {
return nil, err
}

if !templateConf.IsSet("type") {
return nil, fmt.Errorf("template %v: must have a 'type'", uri)
}
templateType, ok := templateConf.Get("type").(string)
if !ok {
return nil, fmt.Errorf("template %v: 'type' must be a string", uri)
}

if !templateConf.IsSet("template") {
return nil, fmt.Errorf("template %v: must have a 'template'", uri)
}

rawTemplate, ok := templateConf.Get("template").(string)
if !ok {
return nil, fmt.Errorf("template %v: 'template' must be a string", uri)
}

if _, err = template.New(templateType).Parse(rawTemplate); err != nil {
return nil, fmt.Errorf("template %v: parse as text/template: %w", uri, err)
}

return confmap.NewRetrieved(map[string]any{
allTemplatesKey: map[string]any{
templateType: rawTemplate,
},
})
}

func (*provider) Scheme() string {
return schemeName
}

func (*provider) Shutdown(context.Context) error {
return nil
}
145 changes: 145 additions & 0 deletions confmap/provider/templateprovider/provider_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package templateprovider

import (
"context"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/confmap"
"go.opentelemetry.io/collector/confmap/confmaptest"
)

const templateSchemePrefix = schemeName + ":"

func TestValidateProviderScheme(t *testing.T) {
assert.NoError(t, confmaptest.ValidateProviderScheme(New()))
}

func TestEmptyName(t *testing.T) {
fp := New()
_, err := fp.Retrieve(context.Background(), "", nil)
require.Error(t, err)
require.NoError(t, fp.Shutdown(context.Background()))
}

func TestUnsupportedScheme(t *testing.T) {
fp := New()
_, err := fp.Retrieve(context.Background(), "https://", nil)
assert.Error(t, err)
assert.NoError(t, fp.Shutdown(context.Background()))
}

func TestNonExistent(t *testing.T) {
fp := New()
_, err := fp.Retrieve(context.Background(), templateSchemePrefix+filepath.Join("testdata", "non-existent.yaml"), nil)
assert.Error(t, err)
_, err = fp.Retrieve(context.Background(), templateSchemePrefix+absolutePath(t, filepath.Join("testdata", "non-existent.yaml")), nil)
assert.Error(t, err)
require.NoError(t, fp.Shutdown(context.Background()))
}

func TestInvalidYAML(t *testing.T) {
fp := New()
_, err := fp.Retrieve(context.Background(), templateSchemePrefix+filepath.Join("testdata", "invalid-yaml.yaml"), nil)
assert.Error(t, err)
_, err = fp.Retrieve(context.Background(), templateSchemePrefix+absolutePath(t, filepath.Join("testdata", "invalid-yaml.yaml")), nil)
assert.Error(t, err)
require.NoError(t, fp.Shutdown(context.Background()))
}

func TestNonMapContent(t *testing.T) {
fp := New()
_, err := fp.Retrieve(context.Background(), templateSchemePrefix+filepath.Join("testdata", "nonmap.yaml"), nil)
assert.ErrorContains(t, err, "cannot be used as a Conf")
_, err = fp.Retrieve(context.Background(), templateSchemePrefix+absolutePath(t, filepath.Join("testdata", "nonmap.yaml")), nil)
assert.ErrorContains(t, err, "cannot be used as a Conf")
require.NoError(t, fp.Shutdown(context.Background()))
}

func TestMissingType(t *testing.T) {
fp := New()
_, err := fp.Retrieve(context.Background(), templateSchemePrefix+filepath.Join("testdata", "missing-type.yaml"), nil)
assert.ErrorContains(t, err, "must have a 'type'")
_, err = fp.Retrieve(context.Background(), templateSchemePrefix+absolutePath(t, filepath.Join("testdata", "missing-type.yaml")), nil)
assert.ErrorContains(t, err, "must have a 'type'")
require.NoError(t, fp.Shutdown(context.Background()))
}

func TestNonStringType(t *testing.T) {
fp := New()
_, err := fp.Retrieve(context.Background(), templateSchemePrefix+filepath.Join("testdata", "nonstring-type.yaml"), nil)
assert.ErrorContains(t, err, "type' must be a string")
_, err = fp.Retrieve(context.Background(), templateSchemePrefix+absolutePath(t, filepath.Join("testdata", "nonstring-type.yaml")), nil)
assert.ErrorContains(t, err, "type' must be a string")
require.NoError(t, fp.Shutdown(context.Background()))
}

func TestMissingTemplate(t *testing.T) {
fp := New()
_, err := fp.Retrieve(context.Background(), templateSchemePrefix+filepath.Join("testdata", "missing-template.yaml"), nil)
assert.ErrorContains(t, err, "must have a 'template'")
_, err = fp.Retrieve(context.Background(), templateSchemePrefix+absolutePath(t, filepath.Join("testdata", "missing-template.yaml")), nil)
assert.ErrorContains(t, err, "must have a 'template'")
require.NoError(t, fp.Shutdown(context.Background()))
}

func TestNonStringTemplate(t *testing.T) {
fp := New()
_, err := fp.Retrieve(context.Background(), templateSchemePrefix+filepath.Join("testdata", "nonstring-template.yaml"), nil)
assert.ErrorContains(t, err, "template' must be a string")
_, err = fp.Retrieve(context.Background(), templateSchemePrefix+absolutePath(t, filepath.Join("testdata", "nonstring-template.yaml")), nil)
assert.ErrorContains(t, err, "template' must be a string")
require.NoError(t, fp.Shutdown(context.Background()))
}

func TestInvalidTemplate(t *testing.T) {
fp := New()
_, err := fp.Retrieve(context.Background(), templateSchemePrefix+filepath.Join("testdata", "invalid-template.yaml"), nil)
assert.ErrorContains(t, err, "parse as text/template")
_, err = fp.Retrieve(context.Background(), templateSchemePrefix+absolutePath(t, filepath.Join("testdata", "invalid-template.yaml")), nil)
assert.ErrorContains(t, err, "parse as text/template")
require.NoError(t, fp.Shutdown(context.Background()))
}

func TestRelativePath(t *testing.T) {
fp := New()
ret, err := fp.Retrieve(context.Background(), templateSchemePrefix+filepath.Join("testdata", "default-config.yaml"), nil)
require.NoError(t, err)
retMap, err := ret.AsConf()
assert.NoError(t, err)
expectedMap := confmap.NewFromStringMap(map[string]any{
allTemplatesKey: map[string]any{
"my_filelog_template": "filelog:\n include: {{ .my_file }}\n",
},
})
assert.Equal(t, expectedMap, retMap)
assert.NoError(t, fp.Shutdown(context.Background()))
}

func TestAbsolutePath(t *testing.T) {
fp := New()
ret, err := fp.Retrieve(context.Background(), templateSchemePrefix+absolutePath(t, filepath.Join("testdata", "default-config.yaml")), nil)
require.NoError(t, err)
retMap, err := ret.AsConf()
assert.NoError(t, err)
expectedMap := confmap.NewFromStringMap(map[string]any{
allTemplatesKey: map[string]any{
"my_filelog_template": "filelog:\n include: {{ .my_file }}\n",
},
})
assert.Equal(t, expectedMap, retMap)
assert.NoError(t, fp.Shutdown(context.Background()))
}

func absolutePath(t *testing.T, relativePath string) string {
dir, err := os.Getwd()
require.NoError(t, err)
return filepath.Join(dir, relativePath)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type: my_filelog_template
template: |
filelog:
include: {{ .my_file }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type: my_filelog_template
template: |
filelog:
include: {{
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[invalid,
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
type: my_filelog_template
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
template: |
filelog:
include: {{ .my_file }}
2 changes: 2 additions & 0 deletions confmap/provider/templateprovider/testdata/nonmap.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- 123
- 456
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
type: my_filelog_template
template: [123,456]
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type: [123,456]
template: |
filelog:
include: {{ .my_file }}
Loading