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

feat: add an observability operator #1018

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

feat: add an observability operator #1018

wants to merge 2 commits into from

Conversation

sduranc
Copy link
Collaborator

@sduranc sduranc commented Oct 18, 2024

Description

  • It adds an observability operator based on kube builder
  • For the moment we only need to handle prom rules --> azure rule group
  • It uses the azure sdk directly
  • So far it has tests and not e2e tests completely, since this is still a WIP
  • It uses azidentity, because we wanna run it with workload identity
  • Note: this is really a draft, so I haven't taken the time to clean properly - there might be some outdated stuff here and there

Related Issue(s)

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • [] Relevant automated test added (if you find this hard, leave it and we'll help out)
  • All tests run green

Documentation

  • User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

@sduranc sduranc requested a review from a team as a code owner October 18, 2024 08:31
@sduranc sduranc marked this pull request as draft October 18, 2024 08:32
secondsStr := strconv.FormatFloat(totalSeconds, 'f', -1, 64)
builder.WriteString(fmt.Sprintf("%sS", secondsStr))
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I am kinda curious about the edge case of zero duration for Prometheus inputs. Specifically, cases like "0s", "0m", or an empty string. Shouldn't this ideally be parsed as zero duration, resulting in the ISO 8601 representation of PT0S? 🤔

In code it would look like this:

if isoDuration == "P" {
    isoDuration = "PT0S"
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Specifically, cases like "0s", "0m", or an empty string

The first 2 don't make sense in the context of promQL, but we can definitely add them to the tests.

{"1ms", "PT0.001S"},
{"1000ms", "PT1S"},
{"1y2w3d4h5m6s", "P1Y2W3DT4H5M6S"},
{"1y2w3d4h5m6s7ms", "P1Y2W3DT4H5M6.007S"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Regarding this: https://github.com/Altinn/altinn-platform/pull/1018/files#r1812522355

Also another test case may need to get added here:

{"0", "PT0S"},

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Go ahead, you can just add them. See what happens in those tests.

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