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

Wire up support for converting integrations-next configs to flow. #5973

Merged
merged 13 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from 5 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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ Main (unreleased)

- Added support for `loki.write` to flush WAL on agent shutdown. (@thepalbi)

- Add support for `integrations-next` static to flow config conversion. (@erikbaranowski)

- Add support for passing extra arguments to the static converter such as `-config.expand-env`. (@erikbaranowski)

### Bugfixes

- Update `pyroscope.ebpf` to fix a logical bug causing to profile to many kthreads instead of regular processes https://github.com/grafana/pyroscope/pull/2778 (@korniltsev)
Expand Down
23 changes: 21 additions & 2 deletions cmd/internal/flowmode/cmd_convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ func convertCommand() *cobra.Command {
output: "",
sourceFormat: "",
bypassErrors: false,
extraArgs: "",
}

cmd := &cobra.Command{
Expand All @@ -41,7 +42,11 @@ The -f flag can be used to specify the format we are converting from.

The -b flag can be used to bypass errors. Errors are defined as
non-critical issues identified during the conversion where an
output can still be generated.`,
output can still be generated.

The -e flag can be used to pass extra arguments to the converter
which were used by the original format. Multiple arguments can be passed
by separating them with a space.`,
Args: cobra.RangeArgs(0, 1),
SilenceUsage: true,

Expand Down Expand Up @@ -71,6 +76,7 @@ output can still be generated.`,
cmd.Flags().StringVarP(&f.report, "report", "r", f.report, "The filepath and filename where the report is written.")
cmd.Flags().StringVarP(&f.sourceFormat, "source-format", "f", f.sourceFormat, fmt.Sprintf("The format of the source file. Supported formats: %s.", supportedFormatsList()))
cmd.Flags().BoolVarP(&f.bypassErrors, "bypass-errors", "b", f.bypassErrors, "Enable bypassing errors when converting")
cmd.Flags().StringVarP(&f.extraArgs, "extra-args", "e", f.extraArgs, "Extra arguments from the original format used by the converter. Multiple arguments can be passed by separating them with a space.")
return cmd
}

Expand All @@ -79,6 +85,7 @@ type flowConvert struct {
report string
sourceFormat string
bypassErrors bool
extraArgs string
}

func (fc *flowConvert) Run(configFile string) error {
Expand Down Expand Up @@ -112,7 +119,7 @@ func convert(r io.Reader, fc *flowConvert) error {
return err
}

riverBytes, diags := converter.Convert(inputBytes, converter.Input(fc.sourceFormat), []string{})
riverBytes, diags := converter.Convert(inputBytes, converter.Input(fc.sourceFormat), parseExtraArgs(fc.extraArgs))
err = generateConvertReport(diags, fc)
if err != nil {
return err
Expand Down Expand Up @@ -174,3 +181,15 @@ func supportedFormatsList() string {
}
return strings.Join(ret, ", ")
}

func parseExtraArgs(extraArgs string) []string {
var result []string
if extraArgs != "" {
arguments := strings.Fields(extraArgs)
for _, arg := range arguments {
parts := strings.Split(arg, "=")
result = append(result, parts...)
}
thampiotr marked this conversation as resolved.
Show resolved Hide resolved
}
return result
}
8 changes: 5 additions & 3 deletions cmd/internal/flowmode/cmd_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ depending on the nature of the reload error.
BoolVar(&r.disableReporting, "disable-reporting", r.disableReporting, "Disable reporting of enabled components to Grafana.")
cmd.Flags().StringVar(&r.configFormat, "config.format", r.configFormat, fmt.Sprintf("The format of the source file. Supported formats: %s.", supportedFormatsList()))
cmd.Flags().BoolVar(&r.configBypassConversionErrors, "config.bypass-conversion-errors", r.configBypassConversionErrors, "Enable bypassing errors when converting")
cmd.Flags().StringVar(&r.configExtraArgs, "config.extra-args", r.configExtraArgs, "Extra arguments from the original format used by the converter. Multiple arguments can be passed by separating them with a space.")
return cmd
}

Expand All @@ -144,6 +145,7 @@ type flowRun struct {
clusterName string
configFormat string
configBypassConversionErrors bool
configExtraArgs string
}

func (fr *flowRun) Run(configPath string) error {
Expand Down Expand Up @@ -263,7 +265,7 @@ func (fr *flowRun) Run(configPath string) error {

ready = f.Ready
reload = func() (*flow.Source, error) {
flowSource, err := loadFlowSource(configPath, fr.configFormat, fr.configBypassConversionErrors)
flowSource, err := loadFlowSource(configPath, fr.configFormat, fr.configBypassConversionErrors, fr.configExtraArgs)
defer instrumentation.InstrumentSHA256(flowSource.SHA256())
defer instrumentation.InstrumentLoad(err == nil)

Expand Down Expand Up @@ -362,7 +364,7 @@ func getEnabledComponentsFunc(f *flow.Flow) func() map[string]interface{} {
}
}

func loadFlowSource(path string, converterSourceFormat string, converterBypassErrors bool) (*flow.Source, error) {
func loadFlowSource(path string, converterSourceFormat string, converterBypassErrors bool, configExtraArgs string) (*flow.Source, error) {
fi, err := os.Stat(path)
if err != nil {
return nil, err
Expand Down Expand Up @@ -403,7 +405,7 @@ func loadFlowSource(path string, converterSourceFormat string, converterBypassEr
}
if converterSourceFormat != "flow" {
var diags convert_diag.Diagnostics
bb, diags = converter.Convert(bb, converter.Input(converterSourceFormat), []string{})
bb, diags = converter.Convert(bb, converter.Input(converterSourceFormat), parseExtraArgs(configExtraArgs))
hasError := hasErrorLevel(diags, convert_diag.SeverityLevelError)
hasCritical := hasErrorLevel(diags, convert_diag.SeverityLevelCritical)
if hasCritical || (!converterBypassErrors && hasError) {
Expand Down
2 changes: 1 addition & 1 deletion converter/internal/staticconvert/staticconvert.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func Convert(in []byte, extraArgs []string) ([]byte, diag.Diagnostics) {
var diags diag.Diagnostics

fs := flag.NewFlagSet("convert", flag.ContinueOnError)
args := []string{"-config.file", "convert", "-config.expand-env"}
args := []string{"-config.file", "convert"}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove this hard coded since we can now pass this in and have more control over the converter. This was the lesser of 2 evils before to automatically expand environment vars.

args = append(args, extraArgs...)
staticConfig, err := config.LoadFromFunc(fs, args, func(_, _ string, expandEnvVars bool, c *config.Config) error {
return config.LoadBytes(in, expandEnvVars, c)
Expand Down
4 changes: 2 additions & 2 deletions converter/internal/staticconvert/staticconvert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import (
)

func TestConvert(t *testing.T) {
test_common.TestDirectory(t, "testdata", ".yaml", true, []string{}, staticconvert.Convert)
test_common.TestDirectory(t, "testdata-v2", ".yaml", true, []string{"-enable-features", "integrations-next"}, staticconvert.Convert)
test_common.TestDirectory(t, "testdata", ".yaml", true, []string{"-config.expand-env"}, staticconvert.Convert)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is no longer hard coded to be added so we need to pass it in for the tests.

test_common.TestDirectory(t, "testdata-v2", ".yaml", true, []string{"-enable-features", "integrations-next", "-config.expand-env"}, staticconvert.Convert)

if runtime.GOOS == "windows" {
test_common.TestDirectory(t, "testdata_windows", ".yaml", true, []string{}, staticconvert.Convert)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ loki.write "logs_log_config" {
external_labels = {}
}

logging {
mattdurham marked this conversation as resolved.
Show resolved Hide resolved
level = "debug"
format = "json"
}

prometheus.exporter.azure "integrations_azure1" {
subscriptions = ["subId"]
resource_type = "Microsoft.Dashboard/grafana"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
server:
log_level: ${SOME_ENVIRONMENT_VARIABLE:='debug'}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an integration-next test for env vars to validate it also works.

log_format: json

metrics:
global:
remote_write:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ Configuration conversion is done on a best-effort basis. {{< param "PRODUCT_ROOT

After the configuration is converted, review the {{< param "PRODUCT_NAME" >}} configuration file created and verify that it's correct before starting to use it in a production environment.

Review the following checklist:
This following list of limitations is specific to the converter tooling and not {{< param "PRODUCT_NAME" >}}:
erikbaranowski marked this conversation as resolved.
Show resolved Hide resolved

* The following configurations aren't available for conversion to {{< param "PRODUCT_NAME" >}}: `rule_files`, `alerting`, `remote_read`, `storage`, and `tracing`.
Any additional unsupported features are returned as errors during conversion.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ Configuration conversion is done on a best-effort basis. {{< param "PRODUCT_ROOT

After the configuration is converted, review the {{< param "PRODUCT_NAME" >}} configuration file created and verify that it's correct before starting to use it in a production environment.

Review the following checklist:
This following list of limitations is specific to the converter tooling and not {{< param "PRODUCT_NAME" >}}:
erikbaranowski marked this conversation as resolved.
Show resolved Hide resolved

* Check if you are using any extra command line arguments with Promtail that aren't present in your configuration file. For example, `-max-line-size`.
* Check if you are setting any environment variables, whether [expanded in the config file][] itself or consumed directly by Promtail, such as `JAEGER_AGENT_HOST`.
Expand Down
30 changes: 28 additions & 2 deletions docs/sources/flow/getting-started/migrating-from-static.md
Original file line number Diff line number Diff line change
Expand Up @@ -297,15 +297,41 @@ loki.write "logs_varlogs" {
}
```

## Integrations Next

[Integrations next][] configs can be converter or run by adding the `extra-args` flag for [convert][] or `config.extra-args` for [run][].
erikbaranowski marked this conversation as resolved.
Show resolved Hide resolved

{{< code >}}

```static-binary
AGENT_MODE=flow grafana-agent convert --source-format=static --extra-args="-enable-features=integrations-next" --output=<OUTPUT_CONFIG_PATH> <INPUT_CONFIG_PATH>
```

```flow-binary
grafana-agent-flow convert --source-format=static --extra-args="-enable-features=integrations-next" --output=<OUTPUT_CONFIG_PATH> <INPUT_CONFIG_PATH>
```

{{< /code >}}

erikbaranowski marked this conversation as resolved.
Show resolved Hide resolved
## Environment Vars

You can interpret env variables in your static config using the
`-config.expand-env` command line flag. This can be passed to [convert][] by
including `--extra-args="-config.expand-env"` or to [run][]
by including `--config.extra-args="-config.expand-env"`.
erikbaranowski marked this conversation as resolved.
Show resolved Hide resolved

> It is possible to combine `integrations-next` with `expand-env`.
> For [convert][], `--extra-args="-enable-features=integrations-next -config.expand-env"`
erikbaranowski marked this conversation as resolved.
Show resolved Hide resolved

## Limitations

Configuration conversion is done on a best-effort basis. {{< param "PRODUCT_ROOT_NAME" >}} will issue warnings or errors where the conversion can't be performed.

After the configuration is converted, review the {{< param "PRODUCT_NAME" >}} configuration file and verify that it's correct before starting to use it in a production environment.

Review the following checklist:
This following list of limitations is specific to the converter tooling and not {{< param "PRODUCT_NAME" >}}:
erikbaranowski marked this conversation as resolved.
Show resolved Hide resolved

* The following configuration options are not available for conversion to {{< param "PRODUCT_NAME" >}}: [Integrations next][], [Traces][], and [Agent Management][].
* The following configuration options are not available for conversion to {{< param "PRODUCT_NAME" >}}: [Traces][] and [Agent Management][].
thampiotr marked this conversation as resolved.
Show resolved Hide resolved
erikbaranowski marked this conversation as resolved.
Show resolved Hide resolved
Any additional unsupported features are returned as errors during conversion.
* There is no gRPC server to configure for {{< param "PRODUCT_NAME" >}}, as any non-default configuration will show as unsupported during the conversion.
* Check if you are using any extra command line arguments with Static that aren't present in your configuration file. For example, `-server.http.address`.
Expand Down
9 changes: 9 additions & 0 deletions docs/sources/flow/reference/cli/convert.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ The following flags are supported:

* `--bypass-errors`, `-b`: Enable bypassing errors when converting.

* `--extra-args`, `e`: Extra arguments from the original format used by the converter.

[prometheus]: #prometheus
[promtail]: #promtail
[static]: #static
Expand Down Expand Up @@ -101,6 +103,13 @@ Refer to [Migrate from Promtail to {{< param "PRODUCT_NAME" >}}]({{< relref "../
Using the `--source-format=static` will convert the source configuration from a
[Grafana Agent Static]({{< relref "../../../static" >}}) configuration to a {{< param "PRODUCT_NAME" >}} configuration.

Include `--extra-args` for passing additional command line flags from the original format.
For example, `--extra-args="-enable-features=integrations-next"` will convert a static
erikbaranowski marked this conversation as resolved.
Show resolved Hide resolved
[integrations-next]({{< relref "../../../static/configuration/integrations/integrations-next/" >}})
configuration to a {{< param "PRODUCT_NAME" >}} configuration. You can also
expand env vars by including the `--extra-args="-config.expand-env"`. Multiple command line
erikbaranowski marked this conversation as resolved.
Show resolved Hide resolved
flags can be combined with a space between, such as `--extra-args="-enable-features=integrations-next -config.expand-env"`.
erikbaranowski marked this conversation as resolved.
Show resolved Hide resolved

If you have unsupported features in a Static mode source configuration, you will receive [errors][] when you convert to a Flow mode configuration. The converter will
also raise warnings for configuration options that may require your attention.

Expand Down
7 changes: 6 additions & 1 deletion docs/sources/flow/reference/cli/run.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ The following flags are supported:
* `--cluster.name`: Name to prevent nodes without this identifier from joining the cluster (default `""`).
* `--config.format`: The format of the source file. Supported formats: `flow`, `prometheus`, `promtail`, `static` (default `"flow"`).
* `--config.bypass-conversion-errors`: Enable bypassing errors when converting (default `false`).
* `--config.extra-args`: Extra arguments from the original format used by the converter.

[in-memory HTTP traffic]: {{< relref "../../concepts/component_controller.md#in-memory-traffic" >}}
[data collection]: {{< relref "../../../data-collection" >}}
Expand Down Expand Up @@ -180,11 +181,15 @@ the source format to River and immediately starts running with the new
configuration. This conversion uses the converter API described in the
[grafana-agent-flow convert][] docs.

If you also use the `--config.bypass-conversion-errors` command-line argument,
If you include the `--config.bypass-conversion-errors` command-line argument,
{{< param "PRODUCT_NAME" >}} will ignore any errors from the converter. Use this argument
with caution because the resulting conversion may not be equivalent to the
original configuration.

Include `--config.extra-args` for passing additional command line flags from
the original format to the converter. See [grafana-agent-flow convert][]
for more details on how `extra-args` work.
erikbaranowski marked this conversation as resolved.
Show resolved Hide resolved

[grafana-agent-flow convert]: {{< relref "./convert.md" >}}
[clustering]: {{< relref "../../concepts/clustering.md" >}}
[go-discover]: https://github.com/hashicorp/go-discover