diff --git a/.github/workflows/test-metrics.yml b/.github/workflows/test-metrics.yml index e3b0a8b..5d6d141 100644 --- a/.github/workflows/test-metrics.yml +++ b/.github/workflows/test-metrics.yml @@ -32,6 +32,7 @@ jobs: if: matrix.configuration == 'cw-agent-stopped' run: sudo /opt/aws/amazon-cloudwatch-agent/bin/amazon-cloudwatch-agent-ctl -a stop || true - uses: ./ + id: runs_on with: metrics: cpu,network,memory,disk,io - name: Setup @@ -56,4 +57,73 @@ jobs: sleep 10 done & - wait \ No newline at end of file + wait + - name: Verify VolumeId disk metric + env: + VOLUME_ID: ${{ steps.runs_on.outputs.volume_id }} + run: | + set -euo pipefail + + if [[ -z "$VOLUME_ID" ]]; then + echo "::error::The action did not resolve an EBS VolumeId" + exit 1 + fi + + # Match the action's credential isolation so preconfigured workflow credentials cannot query a different account. + unset AWS_ACCESS_KEY_ID AWS_SECRET_ACCESS_KEY AWS_SESSION_TOKEN AWS_PROFILE AWS_DEFAULT_PROFILE + unset AWS_WEB_IDENTITY_TOKEN_FILE AWS_ROLE_ARN AWS_ROLE_SESSION_NAME AWS_EC2_METADATA_DISABLED + + metric_queries=$(jq -nc \ + --arg instance_id "$RUNS_ON_INSTANCE_ID" \ + --arg volume_id "$VOLUME_ID" \ + '[{ + Id: "diskused", + MetricStat: { + Metric: { + Namespace: "CWAgent", + MetricName: "disk_used_percent", + Dimensions: [ + {Name: "InstanceId", Value: $instance_id}, + {Name: "VolumeId", Value: $volume_id}, + {Name: "fstype", Value: "ext4"}, + {Name: "path", Value: "/"} + ] + }, + Period: 10, + Stat: "Average" + }, + ReturnData: true + }]') + + for attempt in {1..6}; do + start_time=$(date -u -d '10 minutes ago' +%Y-%m-%dT%H:%M:%SZ) + end_time=$(date -u +%Y-%m-%dT%H:%M:%SZ) + if value_count=$(aws cloudwatch get-metric-data \ + --region "$RUNS_ON_AWS_REGION" \ + --metric-data-queries "$metric_queries" \ + --start-time "$start_time" \ + --end-time "$end_time" \ + --query 'length(MetricDataResults[0].Values)' \ + --output text); then + if [[ "$value_count" =~ ^[1-9][0-9]*$ ]]; then + echo "Verified disk_used_percent for $VOLUME_ID ($value_count datapoints)" + exit 0 + fi + fi + + if [[ "$attempt" -lt 6 ]]; then + sleep 10 + fi + done + + echo "::error::No VolumeId-qualified disk_used_percent datapoints were published for $VOLUME_ID" + while IFS= read -r config_file; do + echo "CloudWatch configuration: $config_file" + cat "$config_file" + done < <(find /tmp "$RUNNER_TEMP" -maxdepth 2 -type f -name 'runs-on-metrics-*.json' 2>/dev/null) + aws cloudwatch list-metrics \ + --region "$RUNS_ON_AWS_REGION" \ + --namespace CWAgent \ + --metric-name disk_used_percent \ + --dimensions Name=InstanceId,Value="$RUNS_ON_INSTANCE_ID" || true + exit 1 diff --git a/action.yml b/action.yml index 62bf3c4..8ca64fc 100644 --- a/action.yml +++ b/action.yml @@ -29,4 +29,8 @@ inputs: sccache: description: 'Enable sccache. Can take either "s3" (RunsOn S3 cache bucket) or be empty (disabled). You still need to setup sccache in your workflow, for instance with mozilla-actions/sccache-action.' required: false - default: '' \ No newline at end of file + default: '' + +outputs: + volume_id: + description: 'EBS volume ID backing the configured disk metrics, when available' diff --git a/internal/monitoring/agent.go b/internal/monitoring/agent.go index 8868708..c6fd89b 100644 --- a/internal/monitoring/agent.go +++ b/internal/monitoring/agent.go @@ -13,6 +13,11 @@ import ( "github.com/sethvargo/go-githubactions" ) +const ( + sysBlockRoot = "/sys/block" + volumeIDStateKey = "disk_volume_id" +) + // https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch-Agent-Configuration-File-Details.html func GenerateCloudWatchConfig(action *githubactions.Action, metrics []string, networkInterface, diskDevice string) error { if len(metrics) == 0 { @@ -27,11 +32,63 @@ func GenerateCloudWatchConfig(action *githubactions.Action, metrics []string, ne // Get network interface and disk device based on config primaryInterface := getNetworkInterface(networkInterface) rootDisk := getDiskDevice(diskDevice) + volumeID := "" + for _, metric := range metrics { + if !strings.EqualFold(metric, "disk") { + continue + } + + resolvedVolumeID, err := getEBSVolumeID(sysBlockRoot, rootDisk) + if err != nil { + action.Warningf("Failed to resolve EBS volume ID for disk device %s: %v", rootDisk, err) + } else { + volumeID = resolvedVolumeID + action.Infof("Using EBS volume ID: %s", volumeID) + } + break + } action.Infof("Using network interface: %s", primaryInterface) action.Infof("Using disk device: %s", rootDisk) - config := CloudWatchConfig{ + config := buildCloudWatchConfig(metrics, primaryInterface, rootDisk, volumeID) + + // Write config file + configFile, err := os.CreateTemp("", "runs-on-metrics-*.json") + if err != nil { + return fmt.Errorf("failed to create temp file: %w", err) + } + configPath := configFile.Name() + defer configFile.Close() + + configJSON, err := json.MarshalIndent(config, "", " ") + if err != nil { + return fmt.Errorf("failed to marshal config: %w", err) + } + + if err := os.WriteFile(configPath, configJSON, 0644); err != nil { + return fmt.Errorf("failed to write config file: %w", err) + } + + action.Infof("Generated CloudWatch config with metrics: %v", metrics) + action.Infof("Config file: %s", configPath) + action.Infof("Config content: %s", string(configJSON)) + + if err := applyCloudWatchConfig(action, configPath); err != nil { + return err + } + + if volumeID != "" { + action.SetOutput("volume_id", volumeID) + // GitHub action setup and post run in separate processes, so persist the exact published dimension. + action.SaveState(volumeIDStateKey, volumeID) + } + + return nil +} + +func buildCloudWatchConfig(metrics []string, primaryInterface, rootDisk, volumeID string) CloudWatchConfig { + cloudWatchConfig := CloudWatchConfig{ Metrics: MetricsConfig{ Namespace: NAMESPACE, MetricsCollected: make(map[string]interface{}), @@ -58,7 +115,7 @@ func GenerateCloudWatchConfig(action *githubactions.Action, metrics []string, ne for _, measurement := range measurements { cpuConfig["measurement"] = append(cpuConfig["measurement"].([]string), measurement.Name) } - config.Metrics.MetricsCollected["cpu"] = cpuConfig + cloudWatchConfig.Metrics.MetricsCollected["cpu"] = cpuConfig case "network": netConfig := map[string]interface{}{ "drop_original_metrics": true, @@ -68,7 +125,7 @@ func GenerateCloudWatchConfig(action *githubactions.Action, metrics []string, ne for _, measurement := range measurements { netConfig["measurement"] = append(netConfig["measurement"].([]string), measurement.Name) } - config.Metrics.MetricsCollected["net"] = netConfig + cloudWatchConfig.Metrics.MetricsCollected["net"] = netConfig case "memory": memConfig := map[string]interface{}{ "drop_original_metrics": true, @@ -77,7 +134,7 @@ func GenerateCloudWatchConfig(action *githubactions.Action, metrics []string, ne for _, measurement := range measurements { memConfig["measurement"] = append(memConfig["measurement"].([]string), measurement.Name) } - config.Metrics.MetricsCollected["mem"] = memConfig + cloudWatchConfig.Metrics.MetricsCollected["mem"] = memConfig case "disk": diskConfig := map[string]interface{}{ "drop_original_metrics": true, @@ -91,7 +148,12 @@ func GenerateCloudWatchConfig(action *githubactions.Action, metrics []string, ne for _, measurement := range measurements { diskConfig["measurement"] = append(diskConfig["measurement"].([]string), measurement.Name) } - config.Metrics.MetricsCollected["disk"] = diskConfig + if volumeID != "" { + diskConfig["append_dimensions"] = map[string]string{ + "VolumeId": volumeID, + } + } + cloudWatchConfig.Metrics.MetricsCollected["disk"] = diskConfig case "io": diskioConfig := map[string]interface{}{ "drop_original_metrics": true, @@ -101,33 +163,11 @@ func GenerateCloudWatchConfig(action *githubactions.Action, metrics []string, ne for _, measurement := range measurements { diskioConfig["measurement"] = append(diskioConfig["measurement"].([]string), measurement.Name) } - config.Metrics.MetricsCollected["diskio"] = diskioConfig + cloudWatchConfig.Metrics.MetricsCollected["diskio"] = diskioConfig } } - // Write config file - configFile, err := os.CreateTemp("", "runs-on-metrics-*.json") - if err != nil { - return fmt.Errorf("failed to create temp file: %w", err) - } - configPath := configFile.Name() - defer configFile.Close() - - configJSON, err := json.MarshalIndent(config, "", " ") - if err != nil { - return fmt.Errorf("failed to marshal config: %w", err) - } - - if err := os.WriteFile(configPath, configJSON, 0644); err != nil { - return fmt.Errorf("failed to write config file: %w", err) - } - - action.Infof("Generated CloudWatch config with metrics: %v", metrics) - action.Infof("Config file: %s", configPath) - action.Infof("Config content: %s", string(configJSON)) - - // Apply the config to the CloudWatch agent (start if needed, or append if already running) - return applyCloudWatchConfig(action, configPath) + return cloudWatchConfig } const agentCtl = "/opt/aws/amazon-cloudwatch-agent/bin/amazon-cloudwatch-agent-ctl" diff --git a/internal/monitoring/agent_test.go b/internal/monitoring/agent_test.go new file mode 100644 index 0000000..98b8684 --- /dev/null +++ b/internal/monitoring/agent_test.go @@ -0,0 +1,35 @@ +package monitoring + +import "testing" + +func TestBuildCloudWatchConfigAddsVolumeIDToDiskMetrics(t *testing.T) { + t.Parallel() + + config := buildCloudWatchConfig([]string{"disk"}, "ens5", "nvme0n1p1", "vol-0123456789abcdef0") + diskConfig := config.Metrics.MetricsCollected["disk"].(map[string]interface{}) + + if got := diskConfig["drop_device"]; got != true { + t.Fatalf("drop_device = %v, want true", got) + } + dimensions, ok := diskConfig["append_dimensions"].(map[string]string) + if !ok { + t.Fatalf("append_dimensions has type %T, want map[string]string", diskConfig["append_dimensions"]) + } + if got := dimensions["VolumeId"]; got != "vol-0123456789abcdef0" { + t.Fatalf("VolumeId = %q, want %q", got, "vol-0123456789abcdef0") + } + if _, exists := dimensions["volume_id"]; exists { + t.Fatal("unexpected lowercase volume_id dimension") + } +} + +func TestBuildCloudWatchConfigOmitsVolumeIDWhenUnavailable(t *testing.T) { + t.Parallel() + + config := buildCloudWatchConfig([]string{"disk"}, "ens5", "nvme0n1p1", "") + diskConfig := config.Metrics.MetricsCollected["disk"].(map[string]interface{}) + + if _, exists := diskConfig["append_dimensions"]; exists { + t.Fatal("append_dimensions should be omitted without a volume ID") + } +} diff --git a/internal/monitoring/helpers.go b/internal/monitoring/helpers.go index 017c2eb..8380ceb 100644 --- a/internal/monitoring/helpers.go +++ b/internal/monitoring/helpers.go @@ -2,15 +2,23 @@ package monitoring import ( "bufio" + "fmt" "math" "os" "os/exec" + "path/filepath" + "regexp" "strings" ) const DEFAULT_NETWORK_INTERFACE = "enp39s0" const DEFAULT_DISK_DEVICE = "nvme0n1p1" +var ( + nvmeDevicePattern = regexp.MustCompile(`^(nvme\d+n\d+)(?:p\d+)?$`) + ebsVolumeIDPattern = regexp.MustCompile(`^vol-?(?:[0-9a-f]{8}|[0-9a-f]{17})$`) +) + // detectPrimaryNetworkInterface finds the primary network interface (excluding loopback and docker) func detectPrimaryNetworkInterface() string { // Try to get the interface used for the default route @@ -110,6 +118,35 @@ func getDiskDevice(diskDevice string) string { return diskDevice } +// getEBSVolumeID resolves the EBS volume backing a Nitro NVMe device. +func getEBSVolumeID(sysBlockRoot, diskDevice string) (string, error) { + // Restrict the user-provided device name before joining it to a privileged sysfs path. + if diskDevice == "" || filepath.Base(diskDevice) != diskDevice { + return "", fmt.Errorf("invalid disk device %q", diskDevice) + } + + matches := nvmeDevicePattern.FindStringSubmatch(diskDevice) + if matches == nil { + return "", fmt.Errorf("disk device %q is not a supported NVMe device", diskDevice) + } + + serialPath := filepath.Join(sysBlockRoot, matches[1], "device", "serial") + serialBytes, err := os.ReadFile(serialPath) + if err != nil { + return "", fmt.Errorf("read EBS volume serial: %w", err) + } + + serial := strings.ToLower(strings.Join(strings.Fields(string(serialBytes)), "")) + if !ebsVolumeIDPattern.MatchString(serial) { + return "", fmt.Errorf("unexpected EBS volume serial %q", serial) + } + if strings.HasPrefix(serial, "vol-") { + return serial, nil + } + + return "vol-" + strings.TrimPrefix(serial, "vol"), nil +} + // calculateStats computes min, max, and average of a slice of floats func calculateStats(data []float64) (min, max, avg float64) { data = sanitizeFloatSeries(data) diff --git a/internal/monitoring/helpers_test.go b/internal/monitoring/helpers_test.go new file mode 100644 index 0000000..311dcb7 --- /dev/null +++ b/internal/monitoring/helpers_test.go @@ -0,0 +1,60 @@ +package monitoring + +import ( + "os" + "path/filepath" + "testing" +) + +func TestGetEBSVolumeID(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + device string + serial string + want string + wantErr bool + writeSerial bool + }{ + {name: "partitioned current ID", device: "nvme0n1p1", serial: "vol0123456789abcdef0\n", want: "vol-0123456789abcdef0", writeSerial: true}, + {name: "unpartitioned current ID", device: "nvme0n1", serial: "vol-0123456789abcdef0", want: "vol-0123456789abcdef0", writeSerial: true}, + {name: "old ID with whitespace", device: "nvme0n1p12", serial: " vol 01234567 \n", want: "vol-01234567", writeSerial: true}, + {name: "invalid serial", device: "nvme0n1p1", serial: "AWS42", wantErr: true, writeSerial: true}, + {name: "missing serial", device: "nvme0n1p1", wantErr: true}, + {name: "unsafe device", device: "../nvme0n1p1", wantErr: true}, + {name: "unsupported device", device: "xvda1", wantErr: true}, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + sysBlockRoot := t.TempDir() + if tt.writeSerial { + serialDir := filepath.Join(sysBlockRoot, "nvme0n1", "device") + if err := os.MkdirAll(serialDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(serialDir, "serial"), []byte(tt.serial), 0o644); err != nil { + t.Fatal(err) + } + } + + got, err := getEBSVolumeID(sysBlockRoot, tt.device) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got volume ID %q", got) + } + return + } + if err != nil { + t.Fatalf("getEBSVolumeID() error = %v", err) + } + if got != tt.want { + t.Fatalf("getEBSVolumeID() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/internal/monitoring/metrics.go b/internal/monitoring/metrics.go index 3c3ead4..2df7e0e 100644 --- a/internal/monitoring/metrics.go +++ b/internal/monitoring/metrics.go @@ -175,12 +175,17 @@ func GenerateMetricsSummary(action *githubactions.Action, metrics []string, form // Get network interface and disk device based on config networkInterface = getNetworkInterface(networkInterface) diskDevice = getDiskDevice(diskDevice) + // The setup process saved the exact custom dimension published by CloudWatch Agent for this post process. + volumeID := savedVolumeID() action.Infof("## CloudWatch Metrics Summary\n") action.Infof("Enabled metrics: %s", strings.Join(metrics, ", ")) action.Infof("Namespace: %s", NAMESPACE) action.Infof("Network interface: %s", networkInterface) action.Infof("Disk device: %s", diskDevice) + if volumeID != "" { + action.Infof("Volume ID: %s", volumeID) + } action.Infof("") showLinks(action, metrics) @@ -215,14 +220,6 @@ func GenerateMetricsSummary(action *githubactions.Action, metrics []string, form } if metricType == "disk" { variants = []string{"/", "/tmp", "/var/lib/docker", "/home/runner"} - dimensions = append(dimensions, types.Dimension{ - Name: aws.String("fstype"), - Value: aws.String("ext4"), - }) - dimensions = append(dimensions, types.Dimension{ - Name: aws.String("path"), - Value: aws.String("/"), - }) } if metricType == "io" { dimensions = append(dimensions, types.Dimension{ @@ -231,10 +228,11 @@ func GenerateMetricsSummary(action *githubactions.Action, metrics []string, form }) } for _, variant := range variants { + queryDimensions := dimensions if metricType == "disk" { - dimensions[len(dimensions)-1].Value = aws.String(variant) + queryDimensions = diskMetricDimensions(variant, volumeID) } - summary := collector.GetMetricSummary(measurement.RealName, NAMESPACE, measurement.Aggregation, dimensions, launchTime) + summary := collector.GetMetricSummary(measurement.RealName, NAMESPACE, measurement.Aggregation, queryDimensions, launchTime) if metricType == "disk" && variant != "/" && summary == nil { continue } @@ -245,6 +243,31 @@ func GenerateMetricsSummary(action *githubactions.Action, metrics []string, form } } +func savedVolumeID() string { + return os.Getenv("STATE_" + volumeIDStateKey) +} + +func diskMetricDimensions(path, volumeID string) []types.Dimension { + dimensions := []types.Dimension{ + { + Name: aws.String("fstype"), + Value: aws.String("ext4"), + }, + { + Name: aws.String("path"), + Value: aws.String(path), + }, + } + if volumeID != "" { + dimensions = append(dimensions, types.Dimension{ + Name: aws.String("VolumeId"), + Value: aws.String(volumeID), + }) + } + + return dimensions +} + // displayMetric shows a metric in the specified format (sparkline or chart) func displayMetric(action *githubactions.Action, name string, summary *MetricSummary, unit string, formatter string, variant string) { if summary == nil { diff --git a/internal/monitoring/metrics_test.go b/internal/monitoring/metrics_test.go index ee18e3e..8a499e9 100644 --- a/internal/monitoring/metrics_test.go +++ b/internal/monitoring/metrics_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/aws/aws-sdk-go-v2/aws" "github.com/sethvargo/go-githubactions" ) @@ -38,3 +39,58 @@ func TestDisplayMetricAllInvalidValuesShowsNoValidData(t *testing.T) { t.Fatalf("expected no-valid-data message, got %q", output.String()) } } + +func TestDiskMetricDimensions(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + volumeID string + want map[string]string + }{ + { + name: "with volume ID", + volumeID: "vol-0123456789abcdef0", + want: map[string]string{ + "fstype": "ext4", + "path": "/tmp", + "VolumeId": "vol-0123456789abcdef0", + }, + }, + { + name: "without volume ID", + want: map[string]string{ + "fstype": "ext4", + "path": "/tmp", + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := make(map[string]string) + for _, dimension := range diskMetricDimensions("/tmp", tt.volumeID) { + got[aws.ToString(dimension.Name)] = aws.ToString(dimension.Value) + } + if len(got) != len(tt.want) { + t.Fatalf("got %d dimensions, want %d: %#v", len(got), len(tt.want), got) + } + for name, wantValue := range tt.want { + if gotValue := got[name]; gotValue != wantValue { + t.Fatalf("dimension %s = %q, want %q", name, gotValue, wantValue) + } + } + }) + } +} + +func TestSavedVolumeID(t *testing.T) { + t.Setenv("STATE_"+volumeIDStateKey, "vol-0123456789abcdef0") + + if got := savedVolumeID(); got != "vol-0123456789abcdef0" { + t.Fatalf("savedVolumeID() = %q, want %q", got, "vol-0123456789abcdef0") + } +} diff --git a/main-linux-amd64 b/main-linux-amd64 index d482fda..fb629ab 100755 Binary files a/main-linux-amd64 and b/main-linux-amd64 differ diff --git a/main-linux-arm64 b/main-linux-arm64 index 33592a6..8d7c608 100755 Binary files a/main-linux-arm64 and b/main-linux-arm64 differ diff --git a/main-windows-amd64.exe b/main-windows-amd64.exe index 69ea072..89f684b 100755 Binary files a/main-windows-amd64.exe and b/main-windows-amd64.exe differ