pyrra-dev / pyrra

Making SLOs with Prometheus manageable, accessible, and easy to use for everyone!
https://demo.pyrra.dev
Apache License 2.0
1.2k stars 107 forks source link

unknown field "latencyNative" in dev.pyrra.v1alpha1.ServiceLevelObjective.spec.indicator #823

Open eslam-gomaa opened 1 year ago

eslam-gomaa commented 1 year ago

I get CRD validation error when using that example pyrra-connect-latency.yaml

error validating data: ValidationError(ServiceLevelObjective.spec.indicator): unknown field "latencyNative" in dev.pyrra.v1alpha1.ServiceLevelObjective.spec.indicator; if you choose to ignore these errors, turn validation off with --validate=false
apiVersion: pyrra.dev/v1alpha1
kind: ServiceLevelObjective
metadata:
  name: kafka-producing-latency
  namespace: kafka-cluster-1
  labels:
    prometheus: k8s
    role: alert-rules
spec:
  target: '99.999'
  window: 1w
  description: test latency
  indicator:
    latencyNative:
      latency: 1ms
      total:
        metric: kafka_producer_request_latency_avg


And for the other way to create latency based SLO, it doesn't work when the metric has an expression is there a way to pass something like this "kafka_producer_request_latency_avg <= 4" as the success metric ?

apiVersion: pyrra.dev/v1alpha1
kind: ServiceLevelObjective
metadata:
  name: kafka-producing-latency
  namespace: kafka-cluster-1
  labels:
    role: alert-rules
    pyrra.dev/team: my-team
spec:
  # alerting:
  #   disabled: true
  target: "99.99"
  window: 1w
  description: "Kafka latency"
  indicator:
    latency:
      success:
-        metric: kafka_producer_request_latency_avg
+        metric: kafka_producer_request_latency_avg <= 4
      total:
        metric: kafka_producer_request_latency_avg

Pyrra version: v0.6.4 helm chart release: 0.8.0

eslam-gomaa commented 1 year ago

seems that the helm chart is not up to date 😕

metalmatze commented 1 year ago

The helm chart is up-to-date. This feature hasn't been released yet and is only available on the main branch. I hope we can get v0.7 out sooner than later.

dotdc commented 1 year ago

@metalmatze Is it intended that latency and latencyNative have different declaration style (success: vs latency:) ? Would be easier to switch from one another if they shared the same style.

metalmatze commented 1 year ago

latency is a two years config at this point whereas latencyNative hasn't even been released. ☺️ Then, the underlying metrics are waaaay different with native histograms now. I can potentially see a future where we use the same configuration. However, we need to keep in mind, that the latency success metric takes the le as a specific string. If that doesn't match it won't work.