From 5969326618ffb5a0e54f2dfd2ab18893114ae34e Mon Sep 17 00:00:00 2001 From: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> Date: Wed, 13 Dec 2023 14:46:46 -0500 Subject: [PATCH 01/11] Wire up support for converting integrations-next configs to flow. Include a mechanism for passing command line flags to the converter from the old format. Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> --- CHANGELOG.md | 4 +++ cmd/internal/flowmode/cmd_convert.go | 22 +++++++++++++-- cmd/internal/flowmode/cmd_run.go | 8 ++++-- .../internal/staticconvert/staticconvert.go | 2 +- .../staticconvert/staticconvert_test.go | 4 +-- .../testdata-v2/integrations_v2.river | 5 ++++ .../testdata-v2/integrations_v2.yaml | 4 +++ .../getting-started/migrating-from-static.md | 28 ++++++++++++++++++- docs/sources/flow/reference/cli/convert.md | 9 ++++++ docs/sources/flow/reference/cli/run.md | 7 ++++- 10 files changed, 83 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95b1b04eb8b6..6fe3f9fde08e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/cmd/internal/flowmode/cmd_convert.go b/cmd/internal/flowmode/cmd_convert.go index be25696d9ccb..59056e1fcd10 100644 --- a/cmd/internal/flowmode/cmd_convert.go +++ b/cmd/internal/flowmode/cmd_convert.go @@ -20,6 +20,7 @@ func convertCommand() *cobra.Command { output: "", sourceFormat: "", bypassErrors: false, + extraArgs: "", } cmd := &cobra.Command{ @@ -41,7 +42,10 @@ 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 used by the original format.`, Args: cobra.RangeArgs(0, 1), SilenceUsage: true, @@ -71,6 +75,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") return cmd } @@ -79,6 +84,7 @@ type flowConvert struct { report string sourceFormat string bypassErrors bool + extraArgs string } func (fc *flowConvert) Run(configFile string) error { @@ -112,7 +118,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 @@ -174,3 +180,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...) + } + } + return result +} diff --git a/cmd/internal/flowmode/cmd_run.go b/cmd/internal/flowmode/cmd_run.go index f73fb09d2ee0..86ec8c051f6a 100644 --- a/cmd/internal/flowmode/cmd_run.go +++ b/cmd/internal/flowmode/cmd_run.go @@ -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") return cmd } @@ -144,6 +145,7 @@ type flowRun struct { clusterName string configFormat string configBypassConversionErrors bool + configExtraArgs string } func (fr *flowRun) Run(configPath string) error { @@ -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) @@ -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 @@ -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) { diff --git a/converter/internal/staticconvert/staticconvert.go b/converter/internal/staticconvert/staticconvert.go index 3b66e2a817fb..5540446b50c1 100644 --- a/converter/internal/staticconvert/staticconvert.go +++ b/converter/internal/staticconvert/staticconvert.go @@ -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"} args = append(args, extraArgs...) staticConfig, err := config.LoadFromFunc(fs, args, func(_, _ string, expandEnvVars bool, c *config.Config) error { return config.LoadBytes(in, expandEnvVars, c) diff --git a/converter/internal/staticconvert/staticconvert_test.go b/converter/internal/staticconvert/staticconvert_test.go index f85e880b37d8..42a79b248eeb 100644 --- a/converter/internal/staticconvert/staticconvert_test.go +++ b/converter/internal/staticconvert/staticconvert_test.go @@ -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) + 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) diff --git a/converter/internal/staticconvert/testdata-v2/integrations_v2.river b/converter/internal/staticconvert/testdata-v2/integrations_v2.river index c5489cbbb1a7..71df30bf7e4e 100644 --- a/converter/internal/staticconvert/testdata-v2/integrations_v2.river +++ b/converter/internal/staticconvert/testdata-v2/integrations_v2.river @@ -16,6 +16,11 @@ loki.write "logs_log_config" { external_labels = {} } +logging { + level = "debug" + format = "json" +} + prometheus.exporter.azure "integrations_azure1" { subscriptions = ["subId"] resource_type = "Microsoft.Dashboard/grafana" diff --git a/converter/internal/staticconvert/testdata-v2/integrations_v2.yaml b/converter/internal/staticconvert/testdata-v2/integrations_v2.yaml index 2fde06c384ae..a335c87de163 100644 --- a/converter/internal/staticconvert/testdata-v2/integrations_v2.yaml +++ b/converter/internal/staticconvert/testdata-v2/integrations_v2.yaml @@ -1,3 +1,7 @@ +server: + log_level: ${SOME_ENVIRONMENT_VARIABLE:='debug'} + log_format: json + metrics: global: remote_write: diff --git a/docs/sources/flow/getting-started/migrating-from-static.md b/docs/sources/flow/getting-started/migrating-from-static.md index 37a350c8d951..80554bb2d645 100644 --- a/docs/sources/flow/getting-started/migrating-from-static.md +++ b/docs/sources/flow/getting-started/migrating-from-static.md @@ -297,6 +297,32 @@ 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][]. + +{{< code >}} + +```static-binary +AGENT_MODE=flow grafana-agent convert --source-format=static --extra-args="-enable-features=integrations-next" --output= +``` + +```flow-binary +grafana-agent-flow convert --source-format=static --extra-args="-enable-features=integrations-next" --output= +``` + +{{< /code >}} + +## 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"`. + +> It is possible to combine `integrations-next` with `expand-env`. +> For [convert][], `--extra-args="-enable-features=integrations-next -config.expand-env"` + ## 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. @@ -305,7 +331,7 @@ After the configuration is converted, review the {{< param "PRODUCT_NAME" >}} co Review the following checklist: -* 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][]. 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`. diff --git a/docs/sources/flow/reference/cli/convert.md b/docs/sources/flow/reference/cli/convert.md index a38b63f7fdb1..f081baab9373 100644 --- a/docs/sources/flow/reference/cli/convert.md +++ b/docs/sources/flow/reference/cli/convert.md @@ -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 @@ -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 +[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 +flags can be combined with a space between, such as `--extra-args="-enable-features=integrations-next -config.expand-env"`. + 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. diff --git a/docs/sources/flow/reference/cli/run.md b/docs/sources/flow/reference/cli/run.md index e65ae0e3d5b0..31b91300627e 100644 --- a/docs/sources/flow/reference/cli/run.md +++ b/docs/sources/flow/reference/cli/run.md @@ -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" >}} @@ -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. + [grafana-agent-flow convert]: {{< relref "./convert.md" >}} [clustering]: {{< relref "../../concepts/clustering.md" >}} [go-discover]: https://github.com/hashicorp/go-discover From 5dde688b98ec87ff1439726dca0236b915b65c7c Mon Sep 17 00:00:00 2001 From: Erik Baranowski <39704712+erikbaranowski@users.noreply.github.com> Date: Thu, 14 Dec 2023 11:07:32 -0500 Subject: [PATCH 02/11] Update cmd/internal/flowmode/cmd_convert.go Co-authored-by: Piotr <17101802+thampiotr@users.noreply.github.com> --- cmd/internal/flowmode/cmd_convert.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/internal/flowmode/cmd_convert.go b/cmd/internal/flowmode/cmd_convert.go index 59056e1fcd10..d55a04954c74 100644 --- a/cmd/internal/flowmode/cmd_convert.go +++ b/cmd/internal/flowmode/cmd_convert.go @@ -45,7 +45,7 @@ non-critical issues identified during the conversion where an output can still be generated. The -e flag can be used to pass extra arguments to the converter -which used by the original format.`, +which were used by the original format.`, Args: cobra.RangeArgs(0, 1), SilenceUsage: true, From e50647f4fd21fbbe55f42c3c96dc6dd3f3f8630d Mon Sep 17 00:00:00 2001 From: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> Date: Thu, 14 Dec 2023 11:19:16 -0500 Subject: [PATCH 03/11] clarify that converter limitations are for the converter and not Grafana Agent Flow Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> --- docs/sources/flow/getting-started/migrating-from-prometheus.md | 2 +- docs/sources/flow/getting-started/migrating-from-promtail.md | 2 +- docs/sources/flow/getting-started/migrating-from-static.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/sources/flow/getting-started/migrating-from-prometheus.md b/docs/sources/flow/getting-started/migrating-from-prometheus.md index ecd287b96116..db76ce6b5fbc 100644 --- a/docs/sources/flow/getting-started/migrating-from-prometheus.md +++ b/docs/sources/flow/getting-started/migrating-from-prometheus.md @@ -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" >}}: * 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. diff --git a/docs/sources/flow/getting-started/migrating-from-promtail.md b/docs/sources/flow/getting-started/migrating-from-promtail.md index 28e3866bbfb7..bedac890ab45 100644 --- a/docs/sources/flow/getting-started/migrating-from-promtail.md +++ b/docs/sources/flow/getting-started/migrating-from-promtail.md @@ -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" >}}: * 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`. diff --git a/docs/sources/flow/getting-started/migrating-from-static.md b/docs/sources/flow/getting-started/migrating-from-static.md index 80554bb2d645..25a9498da4d4 100644 --- a/docs/sources/flow/getting-started/migrating-from-static.md +++ b/docs/sources/flow/getting-started/migrating-from-static.md @@ -329,7 +329,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 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" >}}: * The following configuration options are not available for conversion to {{< param "PRODUCT_NAME" >}}: [Traces][] and [Agent Management][]. Any additional unsupported features are returned as errors during conversion. From cc710b6fff3ae0a10ee8dce23ab66bb229216c04 Mon Sep 17 00:00:00 2001 From: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> Date: Thu, 14 Dec 2023 11:26:29 -0500 Subject: [PATCH 04/11] mention how to pass multiple arguments in the CLI Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> --- cmd/internal/flowmode/cmd_convert.go | 5 +++-- cmd/internal/flowmode/cmd_run.go | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/internal/flowmode/cmd_convert.go b/cmd/internal/flowmode/cmd_convert.go index d55a04954c74..3c44db638144 100644 --- a/cmd/internal/flowmode/cmd_convert.go +++ b/cmd/internal/flowmode/cmd_convert.go @@ -45,7 +45,8 @@ non-critical issues identified during the conversion where an 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.`, +which were used by the original format. Multiple arguments can be passed +by seperating them with a space.`, Args: cobra.RangeArgs(0, 1), SilenceUsage: true, @@ -75,7 +76,7 @@ which were used by the original format.`, 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") + 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 seperating them with a space.") return cmd } diff --git a/cmd/internal/flowmode/cmd_run.go b/cmd/internal/flowmode/cmd_run.go index 86ec8c051f6a..fa72582f7e74 100644 --- a/cmd/internal/flowmode/cmd_run.go +++ b/cmd/internal/flowmode/cmd_run.go @@ -123,7 +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") + 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 seperating them with a space.") return cmd } From 7570e3cd3d16049e863dc8c9dd32c329970fe731 Mon Sep 17 00:00:00 2001 From: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> Date: Thu, 14 Dec 2023 11:56:04 -0500 Subject: [PATCH 05/11] spelling Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> --- cmd/internal/flowmode/cmd_convert.go | 4 ++-- cmd/internal/flowmode/cmd_run.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/internal/flowmode/cmd_convert.go b/cmd/internal/flowmode/cmd_convert.go index 3c44db638144..f014311d133e 100644 --- a/cmd/internal/flowmode/cmd_convert.go +++ b/cmd/internal/flowmode/cmd_convert.go @@ -46,7 +46,7 @@ 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 seperating them with a space.`, +by separating them with a space.`, Args: cobra.RangeArgs(0, 1), SilenceUsage: true, @@ -76,7 +76,7 @@ by seperating them with a space.`, 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 seperating them with a space.") + 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 } diff --git a/cmd/internal/flowmode/cmd_run.go b/cmd/internal/flowmode/cmd_run.go index fa72582f7e74..527254c2632d 100644 --- a/cmd/internal/flowmode/cmd_run.go +++ b/cmd/internal/flowmode/cmd_run.go @@ -123,7 +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 seperating them with a space.") + 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 } From c5db4d6ad89c53277378e6125795a9628a0d10c5 Mon Sep 17 00:00:00 2001 From: Erik Baranowski <39704712+erikbaranowski@users.noreply.github.com> Date: Thu, 14 Dec 2023 13:59:23 -0500 Subject: [PATCH 06/11] Apply suggestions from code review Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com> --- .../migrating-from-prometheus.md | 2 +- .../getting-started/migrating-from-promtail.md | 2 +- .../getting-started/migrating-from-static.md | 18 ++++++++++-------- docs/sources/flow/reference/cli/convert.md | 6 +++--- docs/sources/flow/reference/cli/run.md | 5 ++--- 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/docs/sources/flow/getting-started/migrating-from-prometheus.md b/docs/sources/flow/getting-started/migrating-from-prometheus.md index db76ce6b5fbc..14c8f1eb1fb8 100644 --- a/docs/sources/flow/getting-started/migrating-from-prometheus.md +++ b/docs/sources/flow/getting-started/migrating-from-prometheus.md @@ -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. -This following list of limitations is specific to the converter tooling and not {{< param "PRODUCT_NAME" >}}: +The following list is specific to the convert command and not {{< param "PRODUCT_NAME" >}}: * 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. diff --git a/docs/sources/flow/getting-started/migrating-from-promtail.md b/docs/sources/flow/getting-started/migrating-from-promtail.md index bedac890ab45..bda9033c52d2 100644 --- a/docs/sources/flow/getting-started/migrating-from-promtail.md +++ b/docs/sources/flow/getting-started/migrating-from-promtail.md @@ -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. -This following list of limitations is specific to the converter tooling and not {{< param "PRODUCT_NAME" >}}: +The following list is specific to the convert command and not {{< param "PRODUCT_NAME" >}}: * 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`. diff --git a/docs/sources/flow/getting-started/migrating-from-static.md b/docs/sources/flow/getting-started/migrating-from-static.md index 25a9498da4d4..7dacac30da76 100644 --- a/docs/sources/flow/getting-started/migrating-from-static.md +++ b/docs/sources/flow/getting-started/migrating-from-static.md @@ -313,15 +313,17 @@ grafana-agent-flow convert --source-format=static --extra-args="-enable-features {{< /code >}} + Replace the following: + * _``_: The full path to the [Static][] configuration. + * _``_: The full path to output the {{< param "PRODUCT_NAME" >}} configuration. + ## 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"`. +You can use the `-config.expand-env` command line flag to interpret environment variables in your Grafana Agent Static configuration. +You can pass these flags to [convert][] with `--extra-args="-config.expand-env"` or to [run][] with `--config.extra-args="-config.expand-env"`. -> It is possible to combine `integrations-next` with `expand-env`. -> For [convert][], `--extra-args="-enable-features=integrations-next -config.expand-env"` +> It's possible to combine `integrations-next` with `expand-env`. +> For [convert][], you can use `--extra-args="-enable-features=integrations-next -config.expand-env"` ## Limitations @@ -329,9 +331,9 @@ Configuration conversion is done on a best-effort basis. {{< param "PRODUCT_ROOT 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. -This following list of limitations is specific to the converter tooling and not {{< param "PRODUCT_NAME" >}}: +The following list is specific to the convert command and not {{< param "PRODUCT_NAME" >}}: -* The following configuration options are not available for conversion to {{< param "PRODUCT_NAME" >}}: [Traces][] and [Agent Management][]. +* The [Traces][] and [Agent Management][] configuration options can't be converted to {{< param "PRODUCT_NAME" >}}. 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`. diff --git a/docs/sources/flow/reference/cli/convert.md b/docs/sources/flow/reference/cli/convert.md index f081baab9373..88c9628fde31 100644 --- a/docs/sources/flow/reference/cli/convert.md +++ b/docs/sources/flow/reference/cli/convert.md @@ -104,11 +104,11 @@ 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 +For example, `--extra-args="-enable-features=integrations-next"` will convert a Grafana Agent Static [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 -flags can be combined with a space between, such as `--extra-args="-enable-features=integrations-next -config.expand-env"`. +expand environment variables with `--extra-args="-config.expand-env"`. You can combine multiple command line +flags with a space between each flag, for example `--extra-args="-enable-features=integrations-next -config.expand-env"`. 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. diff --git a/docs/sources/flow/reference/cli/run.md b/docs/sources/flow/reference/cli/run.md index 31b91300627e..4a7adc21b7b2 100644 --- a/docs/sources/flow/reference/cli/run.md +++ b/docs/sources/flow/reference/cli/run.md @@ -186,9 +186,8 @@ If you include the `--config.bypass-conversion-errors` command-line 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. +Include `--config.extra-args` to pass additional command line flags from the original format to the converter. +Refer to [grafana-agent-flow convert][] for more details on how `extra-args` work. [grafana-agent-flow convert]: {{< relref "./convert.md" >}} [clustering]: {{< relref "../../concepts/clustering.md" >}} From d3941328c5b483cf71fc09eee593a1b3a6d1d3ba Mon Sep 17 00:00:00 2001 From: Erik Baranowski <39704712+erikbaranowski@users.noreply.github.com> Date: Thu, 14 Dec 2023 14:35:15 -0500 Subject: [PATCH 07/11] Update docs/sources/flow/getting-started/migrating-from-static.md --- docs/sources/flow/getting-started/migrating-from-static.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sources/flow/getting-started/migrating-from-static.md b/docs/sources/flow/getting-started/migrating-from-static.md index 7dacac30da76..2bac931d998a 100644 --- a/docs/sources/flow/getting-started/migrating-from-static.md +++ b/docs/sources/flow/getting-started/migrating-from-static.md @@ -299,7 +299,7 @@ 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][]. +You can convert [integrations next][] configurations by adding the `extra-args` flag for [convert][] or `config.extra-args` for [run][]. {{< code >}} From 1f089bf6615aafadc5fe480c0bb9a96e5b855510 Mon Sep 17 00:00:00 2001 From: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> Date: Thu, 14 Dec 2023 17:57:26 -0500 Subject: [PATCH 08/11] more robust parsing for extra-args Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> --- cmd/internal/flowmode/cmd_convert.go | 42 +++++++++++++++++---- cmd/internal/flowmode/cmd_convert_test.go | 46 +++++++++++++++++++++++ cmd/internal/flowmode/cmd_run.go | 7 +++- 3 files changed, 86 insertions(+), 9 deletions(-) create mode 100644 cmd/internal/flowmode/cmd_convert_test.go diff --git a/cmd/internal/flowmode/cmd_convert.go b/cmd/internal/flowmode/cmd_convert.go index f014311d133e..68ede0411993 100644 --- a/cmd/internal/flowmode/cmd_convert.go +++ b/cmd/internal/flowmode/cmd_convert.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/grafana/agent/converter" convert_diag "github.com/grafana/agent/converter/diag" @@ -119,7 +120,12 @@ func convert(r io.Reader, fc *flowConvert) error { return err } - riverBytes, diags := converter.Convert(inputBytes, converter.Input(fc.sourceFormat), parseExtraArgs(fc.extraArgs)) + ea, err := parseExtraArgs(fc.extraArgs) + if err != nil { + return err + } + + riverBytes, diags := converter.Convert(inputBytes, converter.Input(fc.sourceFormat), ea) err = generateConvertReport(diags, fc) if err != nil { return err @@ -182,14 +188,34 @@ func supportedFormatsList() string { return strings.Join(ret, ", ") } -func parseExtraArgs(extraArgs string) []string { +func parseExtraArgs(extraArgs string) ([]string, error) { var result []string - if extraArgs != "" { - arguments := strings.Fields(extraArgs) - for _, arg := range arguments { - parts := strings.Split(arg, "=") - result = append(result, parts...) + if extraArgs == "" { + return result, nil + } + + arguments := strings.Fields(extraArgs) + fs := pflag.NewFlagSet("extra-args", pflag.ExitOnError) + for i, arg := range arguments { + var split []string + if arg[1] == '-' { + split := strings.SplitN(arg, "=", 2) + result = append(result, split[0]) + if len(split) == 2 && split[1] != "" { + result = append(result, "") + fs.StringVar(&result[len(result)-1], split[i][2:], result[len(result)-1], "") + } + } else { + split = strings.SplitN(arg, "=", 2) + result = append(result, split[0]) + if len(split) == 2 && split[1] != "" { + result = append(result, "") + fs.StringVarP(&result[len(result)-1], "", split[i][1:], result[len(result)-1], "") + } } } - return result + + fs.ParseErrorsWhitelist.UnknownFlags = true + err := fs.Parse(arguments) + return result, err } diff --git a/cmd/internal/flowmode/cmd_convert_test.go b/cmd/internal/flowmode/cmd_convert_test.go new file mode 100644 index 000000000000..4dabddd4f619 --- /dev/null +++ b/cmd/internal/flowmode/cmd_convert_test.go @@ -0,0 +1,46 @@ +package flowmode + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestParseExtraArgs(t *testing.T) { + type testCase struct { + name string + extraArgs string + expected []string + } + + var testCases = []testCase{ + { + name: "full", + extraArgs: "--key=value", + expected: []string{"--key", "value"}, + }, + { + name: "shorthand", + extraArgs: "-k=value", + expected: []string{"-k", "value"}, + }, + { + name: "bool", + extraArgs: "--boolVariable", + expected: []string{"--boolVariable"}, + }, + { + name: "spaced", + extraArgs: "--key value", + expected: []string{"--key", "value"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + res, err := parseExtraArgs(tc.extraArgs) + require.NoError(t, err) + require.Equal(t, tc.expected, res) + }) + } +} diff --git a/cmd/internal/flowmode/cmd_run.go b/cmd/internal/flowmode/cmd_run.go index 527254c2632d..591dfaea9bc1 100644 --- a/cmd/internal/flowmode/cmd_run.go +++ b/cmd/internal/flowmode/cmd_run.go @@ -405,7 +405,12 @@ func loadFlowSource(path string, converterSourceFormat string, converterBypassEr } if converterSourceFormat != "flow" { var diags convert_diag.Diagnostics - bb, diags = converter.Convert(bb, converter.Input(converterSourceFormat), parseExtraArgs(configExtraArgs)) + ea, err := parseExtraArgs(configExtraArgs) + if err != nil { + return nil, err + } + + bb, diags = converter.Convert(bb, converter.Input(converterSourceFormat), ea) hasError := hasErrorLevel(diags, convert_diag.SeverityLevelError) hasCritical := hasErrorLevel(diags, convert_diag.SeverityLevelCritical) if hasCritical || (!converterBypassErrors && hasError) { From 228e7ba193f1189a47029a4bb64e393ae1f2565e Mon Sep 17 00:00:00 2001 From: Erik Baranowski <39704712+erikbaranowski@users.noreply.github.com> Date: Fri, 15 Dec 2023 10:16:00 -0500 Subject: [PATCH 09/11] Update docs/sources/flow/getting-started/migrating-from-static.md Co-authored-by: Piotr <17101802+thampiotr@users.noreply.github.com> --- docs/sources/flow/getting-started/migrating-from-static.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sources/flow/getting-started/migrating-from-static.md b/docs/sources/flow/getting-started/migrating-from-static.md index b778a7c79409..8c2a3ff88b22 100644 --- a/docs/sources/flow/getting-started/migrating-from-static.md +++ b/docs/sources/flow/getting-started/migrating-from-static.md @@ -333,7 +333,7 @@ After the configuration is converted, review the {{< param "PRODUCT_NAME" >}} co The following list is specific to the convert command and not {{< param "PRODUCT_NAME" >}}: -* The [Traces][] and [Agent Management][] configuration options can't be converted to {{< param "PRODUCT_NAME" >}}. +* The [Traces][] and [Agent Management][] configuration options can't be automatically converted to {{< param "PRODUCT_NAME" >}}. However, traces are fully supported in {{< param "PRODUCT_NAME" >}} and you can build your configuration manually. 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`. From b00d23651f82edc77e18d54c3001226ee04d0ee9 Mon Sep 17 00:00:00 2001 From: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> Date: Fri, 15 Dec 2023 14:37:45 -0500 Subject: [PATCH 10/11] much more robust testing and implementation of parseExtraArgs with code comments Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> --- cmd/internal/flowmode/cmd_convert.go | 57 ++++++++++++++++------- cmd/internal/flowmode/cmd_convert_test.go | 45 ++++++++++++++++-- 2 files changed, 79 insertions(+), 23 deletions(-) diff --git a/cmd/internal/flowmode/cmd_convert.go b/cmd/internal/flowmode/cmd_convert.go index 68ede0411993..9af3d1b1d405 100644 --- a/cmd/internal/flowmode/cmd_convert.go +++ b/cmd/internal/flowmode/cmd_convert.go @@ -195,27 +195,48 @@ func parseExtraArgs(extraArgs string) ([]string, error) { } arguments := strings.Fields(extraArgs) - fs := pflag.NewFlagSet("extra-args", pflag.ExitOnError) - for i, arg := range arguments { - var split []string - if arg[1] == '-' { - split := strings.SplitN(arg, "=", 2) - result = append(result, split[0]) - if len(split) == 2 && split[1] != "" { - result = append(result, "") - fs.StringVar(&result[len(result)-1], split[i][2:], result[len(result)-1], "") + for _, arg := range arguments { + fs := pflag.NewFlagSet("extra-args", pflag.ExitOnError) + fs.ParseErrorsWhitelist.UnknownFlags = true + keyStartIndex := 0 + doParseFlagValue := false + + // Split the argument into key and value. + splitArgs := strings.SplitN(arg, "=", 2) + + // Append the key to the result. + result = append(result, splitArgs[0]) + + // If the flag has a value, add it to the FlagSet for parsing. + if len(splitArgs) == 2 && splitArgs[1] != "" { + doParseFlagValue = true + if arg[1] == '-' { // longhand flag, ie. --flag=value + keyStartIndex = 2 + } else if arg[0] == '-' { // shorthand flag, ie. -f=value + keyStartIndex = 1 + } else { // invalid flag that was split on '=' but has no dashes in the key + return nil, fmt.Errorf("invalid flag found: %s", arg) } - } else { - split = strings.SplitN(arg, "=", 2) - result = append(result, split[0]) - if len(split) == 2 && split[1] != "" { - result = append(result, "") - fs.StringVarP(&result[len(result)-1], "", split[i][1:], result[len(result)-1], "") + } + + if doParseFlagValue { + result = append(result, "") + lastIndex := len(result) - 1 + key := splitArgs[0][keyStartIndex:] + if keyStartIndex == 2 { + fs.StringVar(&result[lastIndex], key, result[lastIndex], "") + } else { + fs.StringVarP(&result[lastIndex], "", key, result[lastIndex], "") } + + // We must parse the flag here because the pointer to the array element + // &result[lastIndex] is overridden by the next iteration of the loop. + // This can be improved if we preallocate the array, however we won't + // know the final length without analyzing the arguments so there + // is some complexity in doing so. + fs.Parse(arguments) } } - fs.ParseErrorsWhitelist.UnknownFlags = true - err := fs.Parse(arguments) - return result, err + return result, nil } diff --git a/cmd/internal/flowmode/cmd_convert_test.go b/cmd/internal/flowmode/cmd_convert_test.go index 4dabddd4f619..571ea3667326 100644 --- a/cmd/internal/flowmode/cmd_convert_test.go +++ b/cmd/internal/flowmode/cmd_convert_test.go @@ -8,14 +8,15 @@ import ( func TestParseExtraArgs(t *testing.T) { type testCase struct { - name string - extraArgs string - expected []string + name string + extraArgs string + expected []string + expectedError string } var testCases = []testCase{ { - name: "full", + name: "longhand", extraArgs: "--key=value", expected: []string{"--key", "value"}, }, @@ -25,20 +26,54 @@ func TestParseExtraArgs(t *testing.T) { expected: []string{"-k", "value"}, }, { - name: "bool", + name: "bool longhand", extraArgs: "--boolVariable", expected: []string{"--boolVariable"}, }, + { + name: "bool shorthand", + extraArgs: "-b", + expected: []string{"-b"}, + }, + { + name: "combo", + extraArgs: "--key=value -k=value --boolVariable -b", + expected: []string{"--key", "value", "-k", "value", "--boolVariable", "-b"}, + }, { name: "spaced", extraArgs: "--key value", expected: []string{"--key", "value"}, }, + { + name: "value with equals", + extraArgs: `--key="foo=bar"`, + expected: []string{"--key", `"foo=bar"`}, + }, + { + name: "no value", + extraArgs: "--key=", + expected: []string{"--key"}, + }, + { + name: "no dashes", + extraArgs: "key", + expected: []string{"key"}, + }, + { + name: "no dashes with value", + extraArgs: "key=value", + expectedError: "invalid flag found: key=value", + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { res, err := parseExtraArgs(tc.extraArgs) + if tc.expectedError != "" { + require.EqualError(t, err, tc.expectedError) + return + } require.NoError(t, err) require.Equal(t, tc.expected, res) }) From 011904a902cb01e63d2ac4ce82d333f2d51d1202 Mon Sep 17 00:00:00 2001 From: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> Date: Fri, 15 Dec 2023 14:39:38 -0500 Subject: [PATCH 11/11] lint Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> --- cmd/internal/flowmode/cmd_convert.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/internal/flowmode/cmd_convert.go b/cmd/internal/flowmode/cmd_convert.go index 9af3d1b1d405..bb7f11cc9c61 100644 --- a/cmd/internal/flowmode/cmd_convert.go +++ b/cmd/internal/flowmode/cmd_convert.go @@ -234,7 +234,10 @@ func parseExtraArgs(extraArgs string) ([]string, error) { // This can be improved if we preallocate the array, however we won't // know the final length without analyzing the arguments so there // is some complexity in doing so. - fs.Parse(arguments) + err := fs.Parse(arguments) + if err != nil { + return nil, err + } } }