Skip to content

Commit

Permalink
[GEN-1286]: Refactor configuration handling to replace deprecated dro…
Browse files Browse the repository at this point in the history
…pdowns with checkboxes for Clickhouse, Gigapipe and Qryn destinations (#2033)

This pull request includes several changes to the configuration handling
and documentation for the Clickhouse and Qryn destinations. The main
focus is on updating deprecated values and improving the handling of
boolean configurations.

### Configuration Handling Improvements:

*
[`common/config/clickhouse.go`](diffhunk://#diff-138cb50de47f4f21f12e6bfd952391ae5cd871bc0d330bab84eb42e7c440a98bL58-R59):
Replaced direct boolean conversion with a helper function
`getBooleanConfig` for `create_schema` configuration.
*
[`common/config/qryn.go`](diffhunk://#diff-332ef1831a065da3170c8cadfdcf5274667681e4bdbd786e278780cc6f054b0dL48-R48):
Updated the `ModifyConfig` function to use pre-parsed configuration
values and replaced string comparisons with boolean values using
`getBooleanConfig`.
[[1]](diffhunk://#diff-332ef1831a065da3170c8cadfdcf5274667681e4bdbd786e278780cc6f054b0dL48-R48)
[[2]](diffhunk://#diff-332ef1831a065da3170c8cadfdcf5274667681e4bdbd786e278780cc6f054b0dL58-R58)
[[3]](diffhunk://#diff-332ef1831a065da3170c8cadfdcf5274667681e4bdbd786e278780cc6f054b0dL129-R130)
*
[`common/config/qryn_oss.go`](diffhunk://#diff-ce2141637cf39bb35c0c6becea7503ead0d028c866a8f8e2bf779a8146037615L26-L28):
Deprecated "Yes/No" values in favor of "true/false" for
`resourceToTelemetryConversion` and `addExporterName` configurations.
*
[`common/config/utils.go`](diffhunk://#diff-3a30bd3819234a1d17d8a057d80ffc71550dab2fc17687b78b40e5c6f49a5136R95-R99):
Added a new utility function `getBooleanConfig` to handle deprecated
boolean configuration values.

### Documentation Updates:

*
[`docs/backends/clickhouse.mdx`](diffhunk://#diff-f9498b059963c2f930f06518c4fced6a78b53b3dbebb4bf5afeee868cacc54a1L38-R39):
Updated documentation to reflect the change from "Create/Skip" to
"true/false" for the `Create Scheme` configuration.
[[1]](diffhunk://#diff-f9498b059963c2f930f06518c4fced6a78b53b3dbebb4bf5afeee868cacc54a1L38-R39)
[[2]](diffhunk://#diff-f9498b059963c2f930f06518c4fced6a78b53b3dbebb4bf5afeee868cacc54a1L57-R58)
[[3]](diffhunk://#diff-f9498b059963c2f930f06518c4fced6a78b53b3dbebb4bf5afeee868cacc54a1L106-R106)
[[4]](diffhunk://#diff-f9498b059963c2f930f06518c4fced6a78b53b3dbebb4bf5afeee868cacc54a1L143-R143)
* `docs/backends/gigapipe.mdx`, `docs/backends/qryn.mdx`: Updated
documentation to reflect the change from "Yes/No" to "true/false" for
boolean configurations.
[[1]](diffhunk://#diff-4abd168fd2d605be33e39a21b60369eab975899d6dc350120027676208823c3fL59-R60)
[[2]](diffhunk://#diff-a50b66d82235440a1e549628a4d45ae127499c15609635959933e1d0eda473ccL52-R53)

### UI and Frontend Adjustments:

*
[`frontend/webapp/hooks/compute-platform/useComputePlatform.ts`](diffhunk://#diff-408f228c0f11c5a04d006fc956ee35e0d67cd74048af12b857d9bedf51ca87e6R48-R68):
Replaced deprecated string values with boolean values for Clickhouse and
Qryn configurations.
*
[`frontend/webapp/hooks/destinations/useDestinationCRUD.ts`](diffhunk://#diff-c7f19ca063b62568e37726473e1b8c265d3309def0d157772951023c66e055b8L78-R86):
Ensured that undefined values are filtered out when creating or updating
destinations.

### YAML Specification Changes:

* `destinations/data/clickhouse.yaml`,
`destinations/data/gigapipe.yaml`, `destinations/data/qryn.yaml`:
Changed component types from dropdown to checkbox for boolean
configurations and updated initial values accordingly.
[[1]](diffhunk://#diff-44f264f92f35b6a7a0bd00f1f77b39e05aaf84c7d5a17ca79b3bd481f2b36c2aL23-R23)
[[2]](diffhunk://#diff-44f264f92f35b6a7a0bd00f1f77b39e05aaf84c7d5a17ca79b3bd481f2b36c2aL42-R46)
[[3]](diffhunk://#diff-02968da745547cb0250c5b8b23d04de88a89e7ea9a1f10916b717736595bac99L38-L51)
[[4]](diffhunk://#diff-1190ae5d806c855e8bd336a40e4063d7d9b558f2b455a9b973f1a33f6a131d1aL36-L49)
  • Loading branch information
BenElferink authored Dec 20, 2024
1 parent 6497397 commit f9b527c
Show file tree
Hide file tree
Showing 12 changed files with 70 additions and 44 deletions.
5 changes: 2 additions & 3 deletions common/config/clickhouse.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,8 @@ func (c *Clickhouse) ModifyConfig(dest ExporterConfigurer, currentConfig *Config
exporterConfig["password"] = clickhousePassword
}

createSchema, exists := dest.GetConfig()[clickhouseCreateSchema]
createSchemaBoolValue := exists && strings.ToLower(createSchema) == "create"
exporterConfig["create_schema"] = createSchemaBoolValue
createSchema := dest.GetConfig()[clickhouseCreateSchema]
exporterConfig["create_schema"] = getBooleanConfig(createSchema, "create")

dbName, exists := dest.GetConfig()[clickhouseDatabaseName]
if !exists {
Expand Down
8 changes: 4 additions & 4 deletions common/config/qryn.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (g *Qryn) ModifyConfig(dest ExporterConfigurer, currentConfig *Config) erro
if conf.passwordFieldName != "" {
passwordPlaceholder = "${" + conf.passwordFieldName + "}"
}
baseURL, err := parseURL(dest.GetConfig()[qrynHost], conf.key, passwordPlaceholder)
baseURL, err := parseURL(conf.host, conf.key, passwordPlaceholder)
if err != nil {
return errors.Join(err, errors.New("invalid qryn endpoint. gateway will not be configured with qryn"))
}
Expand All @@ -55,7 +55,7 @@ func (g *Qryn) ModifyConfig(dest ExporterConfigurer, currentConfig *Config) erro
currentConfig.Exporters[rwExporterName] = GenericMap{
"endpoint": fmt.Sprintf("%s/api/v1/prom/remote/write", baseURL),
"resource_to_telemetry_conversion": GenericMap{
"enabled": dest.GetConfig()[resourceToTelemetryConversion] == "Yes",
"enabled": conf.resourceToTelemetryConversion,
},
}
metricsPipelineName := "metrics/qryn-" + dest.GetID()
Expand Down Expand Up @@ -126,8 +126,8 @@ func (g *Qryn) getConfigs(dest ExporterConfigurer) qrynConf {
return qrynConf{
host: dest.GetConfig()[qrynHost],
key: dest.GetConfig()[qrynAPIKey],
addExporterName: dest.GetConfig()[qrynAddExporterName] == "Yes",
resourceToTelemetryConversion: dest.GetConfig()[resourceToTelemetryConversion] == "Yes",
addExporterName: getBooleanConfig(dest.GetConfig()[qrynAddExporterName], "Yes"),
resourceToTelemetryConversion: getBooleanConfig(dest.GetConfig()[resourceToTelemetryConversion], "Yes"),
secretsOptional: dest.GetConfig()[qrynSecretsOptional] == "1",
passwordFieldName: dest.GetConfig()[qrynPasswordFieldName],
}
Expand Down
14 changes: 12 additions & 2 deletions common/config/qryn_oss.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,19 @@ func (d QrynOssDest) GetConfig() map[string]string {
conf := d.ExporterConfigurer.GetConfig()
conf[qrynHost] = conf[qrynOssHost]
conf[qrynAPIKey] = conf[qrynOssUsername]
conf[resourceToTelemetryConversion] = conf[qrynOssresourceToTelemetryConversion]
// Yes/No are deperecated, use true/false
if conf[qrynOssresourceToTelemetryConversion] == "true" || conf[qrynOssresourceToTelemetryConversion] == "Yes" {
conf[resourceToTelemetryConversion] = "true"
} else {
conf[resourceToTelemetryConversion] = "false"
}
// Yes/No are deperecated, use true/false
if conf[qrynOssAddExporterName] == "true" || conf[qrynOssAddExporterName] == "Yes" {
conf[qrynAddExporterName] = "true"
} else {
conf[qrynAddExporterName] = "false"
}
conf[qrynSecretsOptional] = "1"
conf[qrynAddExporterName] = conf[qrynOssAddExporterName]
conf[qrynPasswordFieldName] = "QRYN_OSS_PASSWORD"
return conf
}
Expand Down
5 changes: 5 additions & 0 deletions common/config/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,8 @@ func urlHostContainsPort(host string) bool {
return strings.Contains(host, ":")
}
}

func getBooleanConfig(currentValue string, deprecatedValue string) bool {
lowerCaseValue := strings.ToLower(currentValue)
return lowerCaseValue == "true" || lowerCaseValue == deprecatedValue
}
9 changes: 3 additions & 6 deletions destinations/data/clickhouse.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ spec:
componentProps:
type: text
required: true
placeholder: "http://host:port"
placeholder: 'http://host:port'
tooltip: 'Clickhouse server address'
- name: CLICKHOUSE_USERNAME
displayName: Username
Expand All @@ -39,14 +39,11 @@ spec:
tooltip: 'If Clickhouse Authentication is used, provide the password'
- name: CLICKHOUSE_CREATE_SCHEME
displayName: Create Scheme
componentType: dropdown
componentType: checkbox
componentProps:
values:
- Create
- Skip
required: true
tooltip: 'Should the destination create the schema for you?'
initialValue: Create
initialValue: true
- name: CLICKHOUSE_DATABASE_NAME
displayName: Database Name
componentType: input
Expand Down
10 changes: 2 additions & 8 deletions destinations/data/gigapipe.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,13 @@ spec:
required: true
- name: QRYN_RESOURCE_TO_TELEMETRY_CONVERSION
displayName: Convert container attributes to labels
componentType: dropdown
componentType: checkbox
componentProps:
values:
- "Yes"
- "No"
required: false
initialValue: Yes
- name: QRYN_ADD_EXPORTER_NAME
displayName: Add exporter name to labels
componentType: dropdown
componentType: checkbox
componentProps:
values:
- "Yes"
- "No"
required: false
initialValue: Yes
10 changes: 2 additions & 8 deletions destinations/data/qryn.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,13 @@ spec:
type: text
- name: QRYN_OSS_RESOURCE_TO_TELEMETRY_CONVERSION
displayName: Convert container attributes to labels
componentType: dropdown
componentType: checkbox
componentProps:
values:
- "Yes"
- "No"
required: false
initialValue: Yes
- name: QRYN_OSS_ADD_EXPORTER_NAME
displayName: Add exporter name to labels
componentType: dropdown
componentType: checkbox
componentProps:
values:
- "Yes"
- "No"
required: false
initialValue: Yes
12 changes: 6 additions & 6 deletions docs/backends/clickhouse.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ The benefit of this option is that you can see the value fast, without needing t
The downside is that the schema may not be optimized for your specific use case, and may make changes more complicated.

To use it:
- Odigos UI - When adding a new ClickHouse destination, select the `Create` Option under the `Create Scheme` field.
- Destination K8s Manifest - Set the `CLICKHOUSE_CREATE_SCHEME` setting to value `Create`.
- Odigos UI - When adding a new ClickHouse destination, select the `Create Scheme` checkbox field.
- Destination K8s Manifest - Set the `CLICKHOUSE_CREATE_SCHEME` setting to value `true`.

The schema which will be used by default can be found [here](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/exporter/clickhouseexporter/example/default_ddl).

Expand All @@ -54,8 +54,8 @@ This option is not recommended for production workloads:
With this option, you are responsible for creating and managing the schema yourself.

To use it:
- Odigos UI - In `Create Scheme` field, select the the `Skip` Option.
- Destination K8s Manifest - Set the `CLICKHOUSE_CREATE_SCHEME` setting to value `Skip`.
- Odigos UI - Unselect the `Create Scheme` checkbox field.
- Destination K8s Manifest - Set the `CLICKHOUSE_CREATE_SCHEME` setting to value `false`.

The benefit of this option is that you have full control over the schema, and can optimize it for your specific use case.

Expand Down Expand Up @@ -103,7 +103,7 @@ These are optional, keep empty if your ClickHouse server does not require authen

### Schema

- Create Schema - Set to `Skip` if you manage your own schema, or `Create` to have Odigos create the schema for you. See [Create Schema](#create-schema) for more details.
- Create Schema - Set to `false` if you manage your own schema, or `true` to have Odigos create the schema for you. See [Create Schema](#create-schema) for more details.
- Database Name (Required) - The name of the Clickhouse Database where the telemetry data will be stored. The default is `otel`. The Database will not be created when not exists, so make sure you have created it before.
- Table Names - Allows you to customize the names of the tables where the telemetry data will be stored. The default is `otel_traces` for traces and `otel_metrics` for metrics.

Expand Down Expand Up @@ -140,7 +140,7 @@ metadata:
namespace: odigos-system
spec:
data:
CLICKHOUSE_CREATE_SCHEME: <Create Scheme [Create, Skip]>
CLICKHOUSE_CREATE_SCHEME: <Create Scheme>
# CLICKHOUSE_USERNAME: <Username>
# Note: The commented fields above are optional.
CLICKHOUSE_DATABASE_NAME: <Database Name>
Expand Down
4 changes: 2 additions & 2 deletions docs/backends/gigapipe.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ metadata:
spec:
data:
QRYN_API_KEY: <API Key>
# QRYN_RESOURCE_TO_TELEMETRY_CONVERSION: <Convert container attributes to labels [Yes, No]>
# QRYN_ADD_EXPORTER_NAME: <Add exporter name to labels [Yes, No]>
# QRYN_RESOURCE_TO_TELEMETRY_CONVERSION: <Convert container attributes to labels>
# QRYN_ADD_EXPORTER_NAME: <Add exporter name to labels>
# Note: The commented fields above are optional.
QRYN_URL: <API Url>
destinationName: qryn
Expand Down
4 changes: 2 additions & 2 deletions docs/backends/qryn.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ spec:
data:
QRYN_OSS_URL: <API Url>
# QRYN_OSS_USERNAME: <Basic auth username>
# QRYN_OSS_RESOURCE_TO_TELEMETRY_CONVERSION: <Convert container attributes to labels [Yes, No]>
# QRYN_OSS_ADD_EXPORTER_NAME: <Add exporter name to labels [Yes, No]>
# QRYN_OSS_RESOURCE_TO_TELEMETRY_CONVERSION: <Convert container attributes to labels>
# QRYN_OSS_ADD_EXPORTER_NAME: <Add exporter name to labels>
# Note: The commented fields above are optional.
destinationName: qryn-oss
# Uncomment the secretRef below if you are using the optional Secret.
Expand Down
21 changes: 21 additions & 0 deletions frontend/webapp/hooks/compute-platform/useComputePlatform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,27 @@ export const useComputePlatform = (): UseComputePlatformHook => {

return { ...item, type };
}),
destinations: data.computePlatform.destinations.map((item) => {
// Replace deprecated string values, with boolean values
const fields =
item.destinationType.type === 'clickhouse'
? item.fields.replace('"CLICKHOUSE_CREATE_SCHEME":"Create"', '"CLICKHOUSE_CREATE_SCHEME":"true"').replace('"CLICKHOUSE_CREATE_SCHEME":"Skip"', '"CLICKHOUSE_CREATE_SCHEME":"false"')
: item.destinationType.type === 'qryn'
? item.fields
.replace('"QRYN_ADD_EXPORTER_NAME":"Yes"', '"QRYN_ADD_EXPORTER_NAME":"true"')
.replace('"QRYN_ADD_EXPORTER_NAME":"No"', '"QRYN_ADD_EXPORTER_NAME":"false"')
.replace('"QRYN_RESOURCE_TO_TELEMETRY_CONVERSION":"Yes"', '"QRYN_RESOURCE_TO_TELEMETRY_CONVERSION":"true"')
.replace('"QRYN_RESOURCE_TO_TELEMETRY_CONVERSION":"No"', '"QRYN_RESOURCE_TO_TELEMETRY_CONVERSION":"false"')
: item.destinationType.type === 'qryn-oss'
? item.fields
.replace('"QRYN_OSS_ADD_EXPORTER_NAME":"Yes"', '"QRYN_OSS_ADD_EXPORTER_NAME":"true"')
.replace('"QRYN_OSS_ADD_EXPORTER_NAME":"No"', '"QRYN_OSS_ADD_EXPORTER_NAME":"false"')
.replace('"QRYN_OSS_RESOURCE_TO_TELEMETRY_CONVERSION":"Yes"', '"QRYN_OSS_RESOURCE_TO_TELEMETRY_CONVERSION":"true"')
.replace('"QRYN_OSS_RESOURCE_TO_TELEMETRY_CONVERSION":"No"', '"QRYN_OSS_RESOURCE_TO_TELEMETRY_CONVERSION":"false"')
: item.fields;

return { ...item, fields };
}),
},
};
}, [data]);
Expand Down
12 changes: 9 additions & 3 deletions frontend/webapp/hooks/destinations/useDestinationCRUD.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,14 @@ export const useDestinationCRUD = (params?: Params) => {
loading: cState.loading || uState.loading || dState.loading,
destinations: data?.computePlatform.destinations || [],

createDestination: (destination: DestinationInput) => createDestination({ variables: { destination: { ...destination, fields: destination.fields.filter(({ value }) => value !== undefined) } } }),
updateDestination: (id: string, destination: DestinationInput) => updateDestination({ variables: { id, destination } }),
deleteDestination: (id: string) => deleteDestination({ variables: { id } }),
createDestination: (destination: DestinationInput) => {
createDestination({ variables: { destination: { ...destination, fields: destination.fields.filter(({ value }) => value !== undefined) } } });
},
updateDestination: (id: string, destination: DestinationInput) => {
updateDestination({ variables: { id, destination: { ...destination, fields: destination.fields.filter(({ value }) => value !== undefined) } } });
},
deleteDestination: (id: string) => {
deleteDestination({ variables: { id } });
},
};
};

0 comments on commit f9b527c

Please sign in to comment.