Verified Commit a1276222 authored by Bob Van Landuyt's avatar Bob Van Landuyt 💬 Committed by GitLab
Browse files

Merge branch 'feat/ux-sli-custom-target' into 'master'

feat: User Experience SLI custom apdex target

Closes gitlab-com/gl-infra/observability/team#4450

See merge request !305

Merged-by: Bob Van Landuyt's avatarBob Van Landuyt <bob@gitlab.com>
Approved-by: Bob Van Landuyt's avatarBob Van Landuyt <bob@gitlab.com>
Reviewed-by: default avatarGitLab Duo <gitlab-duo@gitlab.com>
Co-authored-by: Hercules Merscher's avatarHercules Merscher <hmerscher@gitlab.com>
parents 8bb7514b 5b38e8d4
Loading
Loading
Loading
Loading
Loading
+31 −2
Original line number Diff line number Diff line
@@ -21,15 +21,44 @@
        "sync_fast",
        "sync_slow",
        "async_fast",
        "async_slow"
        "async_slow",
        "custom"
      ],
      "description": "Urgency level for this user experience"
    },
    "apdex_threshold_s": {
      "type": "number",
      "exclusiveMinimum": 0,
      "maximum": 600,
      "description": "Custom Apdex threshold duration in seconds. Required when urgency is custom."
    }
  },
  "required": [
    "description",
    "feature_category",
    "urgency"
  ],
  "allOf": [
    {
      "if": {
        "properties": {
          "urgency": {
            "const": "custom"
          }
        }
      },
      "then": {
        "required": [
          "apdex_threshold_s"
        ]
      },
      "else": {
        "not": {
          "required": [
            "apdex_threshold_s"
          ]
        }
      }
    }
  ]
}
+16 −0
Original line number Diff line number Diff line
@@ -107,6 +107,22 @@ https://docs.gitlab.com/development/feature_categorization/#feature-categorizati
| `sync_slow`  | A user is awaiting a synchronous response which needs to be returned before they can continue with their action, but which the user may accept a slower response | Displaying a full-text search response while displaying an amusement animation | 5s    |
| `async_fast` | An async process which may block a user from continuing with their user journey                                                                                  | MR diff update after git push                                                  | 15s   |
| `async_slow` | An async process which will not block a user and will not be immediately noticed as being slow                                                                   | Notification following an assignment                                           | 5m    |
| `custom`     | A user experience whose acceptable duration does not match the predefined urgency buckets                                                                        | Creating a merge request with an accepted threshold of 90s                     | Set by `apdex_threshold_s` |

**Custom Apdex thresholds**

Use `urgency: "custom"` when none of the predefined thresholds accurately represents the expected user experience. Custom urgency requires `apdex_threshold_s`.

```yaml
description: "Creating a new merge request in a project"
feature_category: "code_review_workflow"
urgency: "custom"
apdex_threshold_s: 90
```

`apdex_threshold_s` is measured in seconds, must be greater than `0`, and cannot exceed `600`.

Prefer the predefined urgency values unless the SLI would otherwise be permanently red or permanently green because the predefined thresholds do not match the experience.

## Usage

+3 −1
Original line number Diff line number Diff line
@@ -209,7 +209,9 @@ module Labkit
      end

      def urgency_threshold
        URGENCY_THRESHOLDS_IN_SECONDS[@definition.urgency.to_sym]
        # Definitions are loaded through the registry, which validates the schema before instantiating them.
        # The schema guarantees experience with custom urgency provide apdex_threshold_s.
        @definition.apdex_threshold_s || URGENCY_THRESHOLDS_IN_SECONDS[@definition.urgency.to_sym]
      end

      def elapsed_time
+8 −3
Original line number Diff line number Diff line
@@ -8,7 +8,11 @@ require 'labkit/json_schema/ref_resolver'

module Labkit
  module UserExperienceSli
    Definition = Data.define(:user_experience_id, :description, :feature_category, :urgency)
    Definition = Data.define(:user_experience_id, :description, :feature_category, :urgency, :apdex_threshold_s) do
      def initialize(user_experience_id:, description:, feature_category:, urgency:, apdex_threshold_s: nil)
        super
      end
    end

    class Registry
      extend Forwardable
@@ -78,15 +82,16 @@ module Labkit
        content = YAML.safe_load(file_path.read)
        return nil unless content.is_a?(Hash)

        return Definition.new(user_experience_id: experience_id, **content) if schema.valid?(content)
        return Definition.new(user_experience_id: experience_id, **content.transform_keys(&:to_sym)) if schema.valid?(content)

        warn("Invalid schema for #{file_path}")

        nil
      rescue Psych::SyntaxError => e
        warn("Invalid definition file #{file_path}: #{e.message}")
        nil
      rescue StandardError => e
        warn("Unexpected error processing #{file_path}: #{e.message}")
        nil
      end

      def schema
+44 −0
Original line number Diff line number Diff line
@@ -446,6 +446,50 @@ RSpec.describe Labkit::UserExperienceSli::Experience, :with_metrics_config do
      end
    end

    context 'with custom urgency' do
      let(:definition) do
        Labkit::UserExperienceSli::Definition.new(
          user_experience_id: 'custom_threshold',
          description: 'Custom Apdex threshold',
          feature_category: 'source_code_management',
          urgency: 'custom',
          apdex_threshold_s: 90
        )
      end

      let(:duration_s) { 60 }

      it 'uses apdex_threshold_s for Apdex success' do
        labels = definition.to_h.slice(:user_experience_id, :feature_category, :urgency)
        observed

        expect(Labkit::Metrics::Client.get(:gitlab_user_experience_apdex_total).get(labels.merge(success: true))).to eq(1)
      end

      it 'uses apdex_threshold_s for Apdex violation' do
        labels = definition.to_h.slice(:user_experience_id, :feature_category, :urgency)
        start_time = Time.now.utc - 91

        experience.observed(start_time: start_time)

        expect(Labkit::Metrics::Client.get(:gitlab_user_experience_apdex_total).get(labels.merge(success: false))).to eq(1)
      end

      it 'logs the custom urgency threshold' do
        expect(Labkit::UserExperienceSli.configuration.logger).to receive(:info)
          .with(hash_including(checkpoint: 'start', urgency: 'custom', urgency_threshold_s: 90))
          .ordered
          .and_call_original

        expect(Labkit::UserExperienceSli.configuration.logger).to receive(:info)
          .with(hash_including(checkpoint: 'end', urgency: 'custom', urgency_threshold_s: 90))
          .ordered
          .and_call_original

        observed
      end
    end

    context 'with reserved keyword validation' do
      %w[test development].each do |env|
        context "when RAILS_ENV is #{env}" do
Loading