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

[GEN-1286]: Refactor configuration handling to replace deprecated dropdowns with checkboxes for Clickhouse, Gigapipe and Qryn destinations #2033

Merged
merged 18 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
5841993
feat: "other agent" for new UI
BenElferink Dec 15, 2024
ca6c1a9
Merge branch 'main' of https://github.com/odigos-io/odigos
BenElferink Dec 15, 2024
e9b0802
Merge branch 'main' of https://github.com/odigos-io/odigos
BenElferink Dec 16, 2024
cf0d60b
Merge branch 'main' of https://github.com/odigos-io/odigos
BenElferink Dec 16, 2024
b23bf99
Merge branch 'main' of https://github.com/odigos-io/odigos
BenElferink Dec 17, 2024
5c51493
Merge branch 'main' of https://github.com/odigos-io/odigos
BenElferink Dec 17, 2024
301025f
Merge branch 'main' of https://github.com/odigos-io/odigos
BenElferink Dec 19, 2024
f54ecd0
Merge branch 'main' of https://github.com/odigos-io/odigos
BenElferink Dec 19, 2024
955b596
Merge branch 'main' of https://github.com/odigos-io/odigos
BenElferink Dec 19, 2024
9272972
Merge branch 'main' of https://github.com/odigos-io/odigos
BenElferink Dec 19, 2024
0f9d51d
Refactor configuration handling to replace deprecated string values w…
BenElferink Dec 19, 2024
0c6ba15
Refactor Clickhouse and Qryn configuration handling to use a dedicate…
BenElferink Dec 19, 2024
bed0739
Refactor destination CRUD operations to ensure consistent handling of…
BenElferink Dec 19, 2024
b7c0866
fix: remove unrelated comment
BenElferink Dec 19, 2024
42461c8
Merge branch 'main' of https://github.com/odigos-io/odigos into gen-1286
BenElferink Dec 19, 2024
48fe66e
Merge branch 'main' into gen-1286
BenElferink Dec 19, 2024
ee77560
Merge branch 'main' into gen-1286
BenElferink Dec 20, 2024
b60f002
Merge branch 'main' into gen-1286
BenElferink Dec 20, 2024
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
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 } });
},
};
};
Loading