Skip to content

Cmk 36785 - #29

Open
DavidGerva wants to merge 2 commits into
Checkmk:masterfrom
DavidGerva:CMK-36785
Open

Cmk 36785#29
DavidGerva wants to merge 2 commits into
Checkmk:masterfrom
DavidGerva:CMK-36785

Conversation

@DavidGerva

Copy link
Copy Markdown
Collaborator

No description provided.

.expect_err("push interval of -1 should fail")
.kind(),
ErrorKind::ValueValidation
);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressing your comment from the last PR

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd do that in a separate patch

- --push-registration-insecure-skip-site-ca-verification
{{- end }}
{{- end }}
{{- if .Values.otel.enabled }}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Following the logic from push mode

# container-level metrics derived from the kubelet data, using OTLP over HTTP.
otel:
# enabled controls whether the OpenTelemetry export is activated or not.
enabled: false

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not sure

@DavidGerva
DavidGerva marked this pull request as ready for review August 11, 2026 13:32
@DavidGerva
DavidGerva requested a review from relrod August 11, 2026 13:32

#[test]
fn push_interval_cannot_be_zero() {
fn otel_push_interval_cannot_be_less_than_one() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this isn't testing --otel-push-interval so I don't think the rename is right

.expect_err("push interval of -1 should fail")
.kind(),
ErrorKind::ValueValidation
);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd do that in a separate patch


/// Push interval in seconds for sending otel metrics.
#[arg(
long = "otel-push-interval",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I recently discovered, you don't need to specify the value to long if it can be derived from the name of the field. If you just specify long (without the = "otel-push-interval") it will default to the correct thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants