Merge branch 'develop' into bolt/opencost-mods

.github/workflows/build-and-publish-develop.yml

@@ -41,5 +41,17 @@ jobs:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           image_tag: ${{ steps.tags.outputs.IMAGE_TAG }}
           release_version: develop-${{ steps.sha.outputs.OC_SHORTHASH }}
+      
+      - name: Install crane
+        uses: imjasonh/setup-crane@v0.4
+
+      - name: Tag and push latest image
+        run: |
+          # Extract the repository part (everything before the last colon)
+          REPO=$(echo "${{ steps.tags.outputs.IMAGE_TAG }}" | sed 's/:.*$//')
+          # Create the new tag
+          NEW_TAG="${REPO}:develop-latest"
+          echo "Copying ${{ steps.tags.outputs.IMAGE_TAG }} to ${NEW_TAG}"
+          crane copy "${{ steps.tags.outputs.IMAGE_TAG }}" "${NEW_TAG}"
 
 

.github/workflows/build-test-image.yml

@@ -0,0 +1,61 @@
+name: Build and Publish Test Image
+
+on:
+  merge_group:
+    types: [checks_requested]
+  pull_request_target:
+    branches:
+      - develop
+
+
+env:
+  REGISTRY: ghcr.io
+
+jobs:
+  check_actor_permissions:
+        runs-on: ubuntu-latest
+        outputs:
+            ismaintainer: ${{ steps.determine-maintainer.outputs.ismaintainer }}
+        steps:
+          - name: Check team membership
+            uses: tspascoal/get-user-teams-membership@v2
+            id: teamAffiliation
+            with:
+              GITHUB_TOKEN: ${{ secrets.ORG_READER_PAT }}
+              username: ${{ github.actor }}
+              organization: opencost
+          - name: determine if actor is a maintainer
+            id: determine-maintainer
+            run: |
+                echo "Actor: ${{ github.actor }}"
+                echo "teams: ${{ join(steps.teamAffiliation.outputs.teams, ',') }}"
+                echo "Is maintainer: ${{ contains(steps.teamAffiliation.outputs.teams, 'OpenCost Maintainers') }}"
+                echo "ismaintainer=${{ contains(steps.teamAffiliation.outputs.teams, 'OpenCost Maintainers') }}" >> $GITHUB_OUTPUT
+      
+  build-and-publish-test-image:
+    runs-on: ubuntu-latest
+    needs: check_actor_permissions
+    if: ${{ (always() && !cancelled()) && ( github.event_name == 'merge_group' || needs.check_actor_permissions.outputs.ismaintainer == 'true') }}
+    permissions:
+      contents: read
+      packages: write
+    steps:
+      - name: Checkout Repo
+        uses: actions/checkout@v4
+        with:
+          ref: ${{ github.event.merge_group.head_sha || github.event.pull_request.head.sha }}
+      - name: Set SHA
+        id: sha
+        run: |
+          echo "OC_SHORTHASH=$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT
+      - name: Set OpenCost Image Tags
+        id: tags
+        run: |
+          echo "IMAGE_TAG=ghcr.io/${{ github.repository_owner }}/opencost:test-${{ steps.sha.outputs.OC_SHORTHASH }}" >> $GITHUB_OUTPUT
+      - name: Build and publish container
+        uses: ./.github/actions/build-container
+        with:
+          actor: ${{ github.actor }}
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          image_tag: ${{ steps.tags.outputs.IMAGE_TAG }}
+          release_version: test-${{ steps.sha.outputs.OC_SHORTHASH }}

.github/workflows/build-test.yaml

@@ -9,6 +9,9 @@ on:
     branches:
       - develop
 
+  merge_group:
+    types: [checks_requested]
+
 jobs:
   validate-protobuf:
     runs-on: ubuntu-latest

.github/workflows/integration-testing.yaml

@@ -0,0 +1,217 @@
+name: Run OpenCost Integration Tests
+
+on:
+    schedule:
+      - cron: '0 14 * * *'
+    push:
+      branches:
+        - develop
+    pull_request_target:
+      branches:
+        - develop
+    merge_group:
+      types: [checks_requested]
+
+concurrency:
+    group: ${{ github.event.merge_group.head.sha || github.event.pull_request.head.sha || github.ref }}-intg-tests
+    cancel-in-progress: false
+
+jobs:
+    check_actor_permissions:
+      runs-on: ubuntu-latest
+      if: ${{ github.event_name == 'pull_request_target' }}
+      outputs:
+          ismaintainer: ${{ steps.determine-maintainer.outputs.ismaintainer }}
+      steps:
+        - name: Check team membership
+          uses: tspascoal/get-user-teams-membership@v2
+          id: teamAffiliation
+          with:
+            GITHUB_TOKEN: ${{ secrets.ORG_READER_PAT }}
+            username: ${{ github.actor }}
+            organization: opencost
+        - name: determine if actor is a maintainer
+          id: determine-maintainer
+          run: |
+              echo "Actor: ${{ github.actor }}"
+              echo "Is maintainer: ${{ contains(steps.teamAffiliation.outputs.teams, 'OpenCost Maintainers') }}"
+              echo "ismaintainer=${{ contains(steps.teamAffiliation.outputs.teams, 'OpenCost Maintainers') }}" >> $GITHUB_OUTPUT
+  
+    noop-tests:
+        needs: check_actor_permissions
+        permissions: {}
+        runs-on: ubuntu-latest
+        if: ${{ (always() && !cancelled()) && github.event_name == 'pull_request_target'  && needs.check_actor_permissions.outputs.ismaintainer == 'false' }}
+        outputs:
+            is_noop: ${{ steps.noop-tests.outputs.is_noop }}
+        steps:
+          - name: Tests Not Needed
+            id: noop-tests
+            run: |
+              echo "integration tests not running because you are not a maintainer. they will run automatically when a PR is merged."
+              echo "is_noop=true" >> $GITHUB_OUTPUT
+    wait_for_image_ready:
+        runs-on: ubuntu-latest
+        permissions: {}
+        needs: check_actor_permissions
+        if: ${{ (always() && !cancelled()) && ( github.event_name == 'push' || github.event_name == 'merge_group' || (github.event_name == 'pull_request_target'  && needs.check_actor_permissions.outputs.ismaintainer == 'true')) }}
+        outputs:
+            IMAGE_TAG: ${{ steps.set_image_tags.outputs.IMAGE_TAG }}
+            NAMESPACE: ${{ steps.set_image_tags.outputs.NAMESPACE }}
+            MAINBRANCH: ${{ steps.set_image_tags.outputs.mainbranch }}
+            passed: ${{ steps.wait_for_image_ready.outputs.passed }}
+        steps:
+          - uses: actions/checkout@v4
+            with:
+              ref: ${{ github.event.merge_group.head.sha || github.event.pull_request.head.sha || github.ref }}
+          - name: Set OC SHA
+            id: sha
+            run: |
+                 echo "OC_SHORTHASH=$(git rev-parse --short HEAD)"
+                 echo "OC_SHORTHASH=$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT
+          - name: Set image tags
+            id: set_image_tags
+            run: |
+                    echo "github.event_name: ${{ github.event_name }}"
+                    if [[ "${{ github.event_name }}" == "merge_group" ]]; then
+                      echo "IMAGE_TAG=ghcr.io/${{ github.repository_owner }}/opencost:test-${{ steps.sha.outputs.OC_SHORTHASH }}" >> $GITHUB_OUTPUT
+                      echo "NAMESPACE=merge-queue-oc-${{ steps.sha.outputs.OC_SHORTHASH }}" >> $GITHUB_OUTPUT
+                      echo "mainbranch=false" >> $GITHUB_OUTPUT
+                    elif [[ "${{ github.event_name }}" == "pull_request_target" ]]; then
+                      echo "building on maintainer pull request branch"
+                      echo "IMAGE_TAG=ghcr.io/${{ github.repository_owner }}/opencost:test-${{ steps.sha.outputs.OC_SHORTHASH }}" >> $GITHUB_OUTPUT
+                      echo "NAMESPACE=pr-${{ github.event.pull_request.number }}-oc-${{ steps.sha.outputs.OC_SHORTHASH }}" >> $GITHUB_OUTPUT
+                      echo "mainbranch=false" >> $GITHUB_OUTPUT
+                    else
+                      echo "building on develop branch"
+                      echo "IMAGE_TAG=ghcr.io/${{ github.repository_owner }}/opencost:develop-${{ steps.sha.outputs.OC_SHORTHASH }}" >> $GITHUB_OUTPUT
+                      echo "NAMESPACE=develop-oc-${{ steps.sha.outputs.OC_SHORTHASH }}" >> $GITHUB_OUTPUT
+                      echo "mainbranch=true" >> $GITHUB_OUTPUT
+                    fi
+
+          - name: Log into ghcr.io
+            uses: docker/login-action@v3
+            with:
+              registry: ghcr.io
+              username: ${{ github.actor }}
+              password: ${{ secrets.GITHUB_TOKEN }}
+
+          - name: wait for docker image to be ready
+            id: wait_for_image_ready
+            run: |
+                max_attempts=100
+                # Loop until the Docker image can be pulled
+                until docker manifest inspect ${{ steps.set_image_tags.outputs.IMAGE_TAG }}; do
+                    echo "Waiting for Docker image ${{ steps.set_image_tags.outputs.IMAGE_TAG }} to be available, $max_attempts tries remain..."
+                    sleep 6
+                    max_attempts=$((max_attempts - 1))
+                    if [[ $max_attempts -eq 0 ]]; then
+                        echo "Docker image ${{ steps.set_image_tags.outputs.IMAGE_TAG }} is not available after 10 minutes. Exiting..."
+                        exit 1
+                    fi
+                done
+
+                echo "Docker image ${{ steps.set_image_tags.outputs.IMAGE_TAG }} is ready!"
+                
+                echo "passed=true" >> $GITHUB_OUTPUT
+                
+    build-test-stack:
+        needs: wait_for_image_ready
+        if: ${{ (always() && !cancelled()) && ( github.event_name == 'push' || github.event_name == 'merge_group' || (github.event_name == 'pull_request_target'  && needs.check_actor_permissions.outputs.ismaintainer == 'true')) }}
+        uses: opencost/opencost-infra/.github/workflows/build-stack.yaml@main
+        secrets: inherit
+        with:
+            oc-container-version: "${{ needs.wait_for_image_ready.outputs.IMAGE_TAG }}"
+            namespace: "${{ needs.wait_for_image_ready.outputs.NAMESPACE }}"
+
+    wait-for-dns:
+        needs: [wait_for_image_ready, build-test-stack]
+        runs-on: ubuntu-latest
+        if: ${{ (always() && !cancelled()) && ( github.event_name == 'push' || github.event_name == 'merge_group' || (github.event_name == 'pull_request_target'  && needs.check_actor_permissions.outputs.ismaintainer == 'true')) }}
+        permissions: {}
+        steps:
+          - name: Wait for DNS to resolve
+            run: |
+              echo "Waiting for ${{ needs.wait_for_image_ready.outputs.NAMESPACE }}.infra.opencost.io to resolve in DNS..."
+              
+              max_attempts=60
+              until host ${{ needs.wait_for_image_ready.outputs.NAMESPACE }}.infra.opencost.io; do
+                echo "DNS not yet resolved for ${{ needs.wait_for_image_ready.outputs.NAMESPACE }}.infra.opencost.io, $max_attempts tries remain..."
+                sleep 10
+                max_attempts=$((max_attempts - 1))
+                if [[ $max_attempts -eq 0 ]]; then
+                  echo "DNS resolution failed for ${{ needs.wait_for_image_ready.outputs.NAMESPACE }}.infra.opencost.io after 10 minutes. Exiting..."
+                  exit 1
+                fi
+              done
+              
+              echo "DNS resolved successfully for ${{ needs.wait_for_image_ready.outputs.NAMESPACE }}.infra.opencost.io!"
+
+    run-tests:
+        needs: [wait_for_image_ready, build-test-stack, wait-for-dns]
+        if: ${{ (always() && !cancelled()) && ( github.event_name == 'push' || github.event_name == 'merge_group' || (github.event_name == 'pull_request_target'  && needs.check_actor_permissions.outputs.ismaintainer == 'true')) }}
+        permissions: {}
+        uses: opencost/opencost-infra/.github/workflows/test-stack.yaml@main
+        secrets: inherit
+        with:
+            namespace: "${{ needs.wait_for_image_ready.outputs.NAMESPACE }}"
+            target_branch: "${{ github.event.pull_request.head.ref || 'main' }}"
+    
+    teardown-test-stack:
+        needs: [wait_for_image_ready, run-tests]
+        if: ${{ (always() && !cancelled()) && ( github.event_name == 'push' || github.event_name == 'merge_group' || (github.event_name == 'pull_request_target'  && needs.check_actor_permissions.outputs.ismaintainer == 'true')) }}
+        uses: opencost/opencost-infra/.github/workflows/destroy-stack.yaml@main
+        secrets: inherit 
+        permissions: {}
+        with:
+            namespace: "${{ needs.wait_for_image_ready.outputs.NAMESPACE }}"
+
+    check-success:
+        needs: [noop-tests, run-tests]
+        permissions: {}
+        runs-on: ubuntu-latest
+        if: ${{ always() }}
+        steps:
+          - name: Check success
+            run: |
+              if [[ "${{ needs.noop-tests.outputs.is_noop }}" == "true" ]]; then
+                echo "No-op tests, skipping success check"
+                exit 0
+              fi
+              
+              if [[ "${{ needs.run-tests.outputs.passed }}" != "true" ]]; then
+                echo "One or more integration tests failed"
+                exit 1
+              fi
+              
+              echo "All integration tests passed"
+              exit 0
+
+    set-labels:
+      needs: [wait_for_image_ready, run-tests]
+      if: ${{ (always() && !cancelled()) && ( github.event_name == 'pull_request_target' && needs.check_actor_permissions.outputs.ismaintainer == 'true') }}
+      runs-on: ubuntu-latest
+      permissions: {}
+      steps:
+        - name: label integration tests failing
+          if: ${{ always()  && contains(needs.*.result, 'failure') && !cancelled()}}
+          uses: andymckay/labeler@1.0.4
+          with:
+            add-labels: "integration tests failed"
+        - uses: mondeja/remove-labels-gh-action@v2
+          if: ${{ always()  && contains(needs.*.result, 'failure') && !cancelled()}}
+          with:
+                token: ${{ secrets.GITHUB_TOKEN }}
+                labels: |
+                  integration tests passed
+        - name: Label integration tests passing
+          if: ${{ always()  && !contains(needs.*.result, 'failure') && !cancelled()}}
+          uses: andymckay/labeler@1.0.4
+          with:
+            add-labels: "integration tests passed"
+        - uses: mondeja/remove-labels-gh-action@v2
+          if: ${{ always()  && !contains(needs.*.result, 'failure') && !cancelled()}}
+          with:
+              token: ${{ secrets.GITHUB_TOKEN }}
+              labels: |
+                integration tests failed

core/pkg/log/log.go

@@ -73,6 +73,10 @@ func SetLogLevel(l string) error {
 	return nil
 }
 
+func Error(msg string) {
+	log.Error().Msg(msg)
+}
+
 func Errorf(format string, a ...interface{}) {
 	log.Error().Msgf(format, a...)
 }
@@ -88,6 +92,10 @@ func DedupedErrorf(logTypeLimit int, format string, a ...interface{}) {
 	}
 }
 
+func Warn(msg string) {
+	log.Warn().Msg(msg)
+}
+
 func Warnf(format string, a ...interface{}) {
 	log.Warn().Msgf(format, a...)
 }
@@ -142,6 +150,10 @@ func Tracef(format string, a ...interface{}) {
 	log.Trace().Msgf(format, a...)
 }
 
+func Fatal(msg string) {
+	log.Fatal().Msg(msg)
+}
+
 func Fatalf(format string, a ...interface{}) {
 	log.Fatal().Msgf(format, a...)
 }

core/pkg/log/log_test.go

@@ -3,10 +3,14 @@ package log
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/rs/zerolog"
+	"github.com/rs/zerolog/log"
+	"github.com/spf13/viper"
 )
 
 func TestGetLogger(t *testing.T) {
@@ -62,3 +66,283 @@ func parseLogMessage(t *testing.T, logMessage string) map[string]interface{} {
 	}
 	return loggedData
 }
+
+func TestInitLogging(t *testing.T) {
+	// Save original logger and restore it after tests
+	originalLogger := log.Logger
+	defer func() {
+		log.Logger = originalLogger
+	}()
+
+	// Test cases
+	tests := []struct {
+		name                   string
+		format                 string
+		level                  string
+		disableColor           bool
+		showLogLevelSetMessage bool
+		expectedLevel          zerolog.Level
+	}{
+		{
+			name:                   "default settings",
+			format:                 "pretty",
+			level:                  "info",
+			disableColor:           false,
+			showLogLevelSetMessage: true,
+			expectedLevel:          zerolog.InfoLevel,
+		},
+		{
+			name:                   "json format",
+			format:                 "json",
+			level:                  "debug",
+			disableColor:           false,
+			showLogLevelSetMessage: false,
+			expectedLevel:          zerolog.DebugLevel,
+		},
+		{
+			name:                   "invalid level",
+			format:                 "pretty",
+			level:                  "invalid",
+			disableColor:           false,
+			showLogLevelSetMessage: false,
+			expectedLevel:          zerolog.InfoLevel,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			viper.Set(flagFormat, tt.format)
+			viper.Set(flagLevel, tt.level)
+			viper.Set(flagDisableColor, tt.disableColor)
+
+			InitLogging(tt.showLogLevelSetMessage)
+
+			if zerolog.GlobalLevel() != tt.expectedLevel {
+				t.Errorf("expected level %v, got %v", tt.expectedLevel, zerolog.GlobalLevel())
+			}
+		})
+	}
+}
+
+func TestLogLevelManagement(t *testing.T) {
+	// Save original logger and restore it after tests
+	originalLogger := log.Logger
+	defer func() {
+		log.Logger = originalLogger
+	}()
+
+	tests := []struct {
+		name        string
+		level       string
+		expectError bool
+	}{
+		{"valid level - debug", "debug", false},
+		{"valid level - info", "info", false},
+		{"valid level - warn", "warn", false},
+		{"valid level - error", "error", false},
+		{"invalid level", "invalid", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := SetLogLevel(tt.level)
+			if (err != nil) != tt.expectError {
+				t.Errorf("SetLogLevel() error = %v, expectError %v", err, tt.expectError)
+			}
+
+			if !tt.expectError {
+				currentLevel := GetLogLevel()
+				if currentLevel != tt.level {
+					t.Errorf("GetLogLevel() = %v, want %v", currentLevel, tt.level)
+				}
+			}
+		})
+	}
+}
+
+func TestLoggingFunctions(t *testing.T) {
+	// Create a buffer to capture log output
+	var buf bytes.Buffer
+	logger := zerolog.New(&buf)
+	SetLogger(&logger)
+
+	// Set log level to trace to ensure all messages are captured
+	zerolog.SetGlobalLevel(zerolog.TraceLevel)
+
+	tests := []struct {
+		name     string
+		logFunc  func(string)
+		logMsg   string
+		expected string
+	}{
+		{"Error", Error, "test error", "error"},
+		{"Warn", Warn, "test warning", "warn"},
+		{"Info", Info, "test info", "info"},
+		{"Debug", Debug, "test debug", "debug"},
+		{"Trace", Trace, "test trace", "trace"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			buf.Reset()
+			tt.logFunc(tt.logMsg)
+			output := buf.String()
+			if !strings.Contains(output, tt.expected) {
+				t.Errorf("expected log level %s in output, got: %s", tt.expected, output)
+			}
+		})
+	}
+
+	// Test Fatal separately since it calls os.Exit
+	t.Run("Fatal", func(t *testing.T) {
+		// Create a new buffer for Fatal test
+		var fatalBuf bytes.Buffer
+		fatalLogger := zerolog.New(&fatalBuf)
+		SetLogger(&fatalLogger)
+
+		// We can't actually test the Fatal function since it calls os.Exit
+		// Instead, we'll verify that the Fatal function exists by checking its type
+		// and that it can be assigned to a variable of the correct type
+		var fatalFunc func(string) = Fatal
+		if fatalFunc == nil {
+			t.Error("Fatal function is nil")
+		}
+	})
+}
+
+func TestDedupedLogging(t *testing.T) {
+	// Create a buffer to capture log output
+	var buf bytes.Buffer
+	logger := zerolog.New(&buf)
+	SetLogger(&logger)
+
+	tests := []struct {
+		name         string
+		logFunc      func(int, string, ...interface{})
+		logTypeLimit int
+		format       string
+		args         []interface{}
+	}{
+		{
+			name:         "DedupedErrorf",
+			logFunc:      DedupedErrorf,
+			logTypeLimit: 3,
+			format:       "test error %d",
+			args:         []interface{}{1},
+		},
+		{
+			name:         "DedupedWarningf",
+			logFunc:      DedupedWarningf,
+			logTypeLimit: 2,
+			format:       "test warning %d",
+			args:         []interface{}{1},
+		},
+		{
+			name:         "DedupedInfof",
+			logFunc:      DedupedInfof,
+			logTypeLimit: 4,
+			format:       "test info %d",
+			args:         []interface{}{1},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			buf.Reset()
+			// Log up to the limit
+			for i := 0; i < tt.logTypeLimit+1; i++ {
+				tt.logFunc(tt.logTypeLimit, tt.format, tt.args...)
+			}
+
+			output := buf.String()
+			// Count occurrences of the exact log message (excluding the suppression message)
+			exactMsg := fmt.Sprintf(tt.format, tt.args...)
+			count := strings.Count(output, exactMsg) - 1 // Subtract 1 for the suppression message
+
+			if count != tt.logTypeLimit {
+				t.Errorf("expected %d occurrences of log message, got %d", tt.logTypeLimit, count)
+			}
+
+			// Verify suppression message
+			suppressionMsg := fmt.Sprintf("%s logged %d times: suppressing future logs", exactMsg, tt.logTypeLimit)
+			if !strings.Contains(output, suppressionMsg) {
+				t.Error("expected suppression message in output")
+			}
+		})
+	}
+}
+
+func TestProfiling(t *testing.T) {
+	// Create a buffer to capture log output
+	var buf bytes.Buffer
+	logger := zerolog.New(&buf)
+	SetLogger(&logger)
+
+	t.Run("Profilef", func(t *testing.T) {
+		buf.Reset()
+		Profilef("test profile %d", 1)
+		output := buf.String()
+		if !strings.Contains(output, "[Profiler]") {
+			t.Error("expected [Profiler] in output")
+		}
+	})
+
+	t.Run("Profile", func(t *testing.T) {
+		buf.Reset()
+		start := time.Now()
+		time.Sleep(10 * time.Millisecond) // Ensure some time has passed
+		Profile(start, "test operation")
+		output := buf.String()
+		if !strings.Contains(output, "test operation") {
+			t.Error("expected operation name in output")
+		}
+	})
+
+	t.Run("ProfileWithThreshold", func(t *testing.T) {
+		buf.Reset()
+		start := time.Now()
+		time.Sleep(10 * time.Millisecond)
+		ProfileWithThreshold(start, 5*time.Millisecond, "test operation")
+		output := buf.String()
+		if !strings.Contains(output, "test operation") {
+			t.Error("expected operation name in output")
+		}
+
+		// Test with threshold not exceeded
+		buf.Reset()
+		start = time.Now()
+		ProfileWithThreshold(start, 100*time.Millisecond, "test operation")
+		output = buf.String()
+		if output != "" {
+			t.Error("expected no output when threshold not exceeded")
+		}
+	})
+}
+
+func TestLoggerManagement(t *testing.T) {
+	// Save original logger and restore it after tests
+	originalLogger := log.Logger
+	defer func() {
+		log.Logger = originalLogger
+	}()
+
+	// Create a new logger with a unique field to identify it
+	var buf bytes.Buffer
+	newLogger := zerolog.New(&buf).With().Str("test_id", "unique_logger").Logger()
+
+	// Test GetLogger
+	currentLogger := GetLogger()
+	if currentLogger == nil {
+		t.Error("GetLogger() returned nil")
+	}
+
+	// Test SetLogger
+	SetLogger(&newLogger)
+
+	// Log a message and verify it contains our unique identifier
+	Info("test message")
+	output := buf.String()
+	if !strings.Contains(output, "unique_logger") {
+		t.Error("SetLogger() did not set the logger correctly")
+	}
+}

core/pkg/protocol/http.go

@@ -1,9 +1,9 @@
 package protocol
 
 import (
-	"fmt"
 	"net/http"
 
+	"github.com/opencost/opencost/core/pkg/log"
 	"github.com/opencost/opencost/core/pkg/util/json"
 	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/proto"
@@ -53,6 +53,25 @@ func (hp HTTPProtocol) InternalServerError(message string) HTTPError {
 	}
 }
 
+func (hp HTTPProtocol) NotImplemented(message string) HTTPError {
+	if message == "" {
+		message = "Not Implemented"
+	}
+	return HTTPError{
+		StatusCode: http.StatusNotImplemented,
+		Body:       message,
+	}
+}
+func (hp HTTPProtocol) Forbidden(message string) HTTPError {
+	if message == "" {
+		message = "Forbidden"
+	}
+	return HTTPError{
+		StatusCode: http.StatusForbidden,
+		Body:       message,
+	}
+}
+
 // NotFound creates a NotFound HTTPError
 func (hp HTTPProtocol) NotFound() HTTPError {
 	return HTTPError{
@@ -85,65 +104,84 @@ func (hp HTTPProtocol) ToResponse(data interface{}, err error) *HTTPResponse {
 		Data: data,
 	}
 }
+func (hp HTTPProtocol) WriteRawOK(w http.ResponseWriter) {
+	w.Header().Set("Content-Type", "application/json")
+	w.Header().Set("Content-Length", "0")
+	w.WriteHeader(http.StatusOK)
+}
+
+func (hp HTTPProtocol) WriteRawNoContent(w http.ResponseWriter) {
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(http.StatusNoContent)
+}
+
+// WriteJSONData uses json content-type and json encoder with no data envelope allowing to remove
+// xss CWE as well as backwards compatibility to exisitng FE expectations
+func (hp HTTPProtocol) WriteJSONData(w http.ResponseWriter, data interface{}) {
+	w.Header().Set("Content-Type", "application/json")
+	status := http.StatusOK
+	w.WriteHeader(status)
+	if err := json.NewEncoder(w).Encode(data); err != nil {
+		log.Error("Failed to encode JSON response: " + err.Error())
+	}
+}
+
+// WriteRawError uses json content-type and outputs raw error message for backwards compatibility to existing
+// frontend expectations.
+func (hp HTTPProtocol) WriteRawError(w http.ResponseWriter, httpStatusCode int, err string) {
+	// I know this isn't json, but its what we've done and don't want to break frontned while we fix CWE
+	w.Header().Set("Content-Type", "application/json")
+	http.Error(w, err, httpStatusCode)
+}
+
+// WriteEncodedError writes an error response in the format of HTTPResponse
+func (hp HTTPProtocol) WriteEncodedError(w http.ResponseWriter, httpStatusCode int, errorResponse interface{}) {
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(httpStatusCode)
+	if err := json.NewEncoder(w).Encode(errorResponse); err != nil {
+		log.Error("Failed to encode error response: " + err.Error())
+	}
+}
 
 // WriteData wraps the data payload in an HTTPResponse and writes the resulting response using the
 // http.ResponseWriter
 func (hp HTTPProtocol) WriteData(w http.ResponseWriter, data interface{}) {
+	w.Header().Set("Content-Type", "application/json")
 	status := http.StatusOK
-	resp, err := json.Marshal(&HTTPResponse{
-		Code: status,
-		Data: data,
-	})
-	if err != nil {
-		status = http.StatusInternalServerError
-		resp, _ = json.Marshal(&HTTPResponse{
-			Code:    status,
-			Message: fmt.Sprintf("Error: %s", err),
-		})
-	}
-
 	w.WriteHeader(status)
-	w.Write(resp)
+	if err := json.NewEncoder(w).Encode(data); err != nil {
+		log.Error("Failed to encode response: " + err.Error())
+	}
 }
 
 // WriteDataWithWarning writes the data payload similiar to WriteData except it provides an additional warning message.
 func (hp HTTPProtocol) WriteDataWithWarning(w http.ResponseWriter, data interface{}, warning string) {
+	w.Header().Set("Content-Type", "application/json")
 	status := http.StatusOK
-	resp, err := json.Marshal(&HTTPResponse{
+	resp := &HTTPResponse{
 		Code:    status,
 		Data:    data,
 		Warning: warning,
-	})
-	if err != nil {
-		status = http.StatusInternalServerError
-		resp, _ = json.Marshal(&HTTPResponse{
-			Code:    status,
-			Message: fmt.Sprintf("Error: %s", err),
-		})
 	}
-
 	w.WriteHeader(status)
-	w.Write(resp)
+	if err := json.NewEncoder(w).Encode(resp); err != nil {
+		log.Error("Failed to encode response with warning: " + err.Error())
+	}
 }
 
 // WriteDataWithMessage writes the data payload similiar to WriteData except it provides an additional string message.
 func (hp HTTPProtocol) WriteDataWithMessage(w http.ResponseWriter, data interface{}, message string) {
+	w.Header().Set("Content-Type", "application/json")
 	status := http.StatusOK
-	resp, err := json.Marshal(&HTTPResponse{
+	resp := &HTTPResponse{
 		Code:    status,
 		Data:    data,
 		Message: message,
-	})
-	if err != nil {
-		status = http.StatusInternalServerError
-		resp, _ = json.Marshal(&HTTPResponse{
-			Code:    status,
-			Message: fmt.Sprintf("Error: %s", err),
-		})
 	}
-
 	w.WriteHeader(status)
-	w.Write(resp)
+	if err := json.NewEncoder(w).Encode(resp); err != nil {
+		log.Error("Failed to encode response with message: " + err.Error())
+	}
 }
 
 // WriteProtoWithMessage uses the protojson package to convert proto3 response to json response and
@@ -151,45 +189,41 @@ func (hp HTTPProtocol) WriteDataWithMessage(w http.ResponseWriter, data interfac
 // EmitUnpopulated to true it returns default values in the Json response payload. If error is
 // encountered it sent InternalServerError and the error why the json conversion failed.
 func (hp HTTPProtocol) WriteProtoWithMessage(w http.ResponseWriter, data proto.Message) {
+	w.Header().Set("Content-Type", "application/json")
 	m := protojson.MarshalOptions{
 		EmitUnpopulated: true,
 	}
 	status := http.StatusOK
-	resp, err := m.Marshal(data)
+	w.WriteHeader(status)
+	b, err := m.Marshal(data)
 	if err != nil {
-		status = http.StatusInternalServerError
-		resp, _ = json.Marshal(&HTTPResponse{
-			Message: fmt.Sprintf("Error: %s", err),
-		})
+		hp.WriteError(w, hp.InternalServerError(err.Error()))
+		log.Error("Failed to marshal proto to json: " + err.Error())
+		return
 	}
 
-	w.WriteHeader(status)
-	w.Write(resp)
+	w.Write(b)
 }
 
 // WriteDataWithMessageAndWarning writes the data payload similiar to WriteData except it provides a warning and additional message string.
 func (hp HTTPProtocol) WriteDataWithMessageAndWarning(w http.ResponseWriter, data interface{}, message string, warning string) {
+	w.Header().Set("Content-Type", "application/json")
 	status := http.StatusOK
-	resp, err := json.Marshal(&HTTPResponse{
+	resp := &HTTPResponse{
 		Code:    status,
 		Data:    data,
 		Message: message,
 		Warning: warning,
-	})
-	if err != nil {
-		status = http.StatusInternalServerError
-		resp, _ = json.Marshal(&HTTPResponse{
-			Code:    status,
-			Message: fmt.Sprintf("Error: %s", err),
-		})
 	}
-
 	w.WriteHeader(status)
-	w.Write(resp)
+	if err := json.NewEncoder(w).Encode(resp); err != nil {
+		log.Error("Failed to encode response with message and warning: " + err.Error())
+	}
 }
 
 // WriteError wraps the HTTPError in a HTTPResponse and writes it via http.ResponseWriter
 func (hp HTTPProtocol) WriteError(w http.ResponseWriter, err HTTPError) {
+	w.Header().Set("Content-Type", "application/json")
 	status := err.StatusCode
 	if status == 0 {
 		status = http.StatusInternalServerError
@@ -200,21 +234,17 @@ func (hp HTTPProtocol) WriteError(w http.ResponseWriter, err HTTPError) {
 		Code:    status,
 		Message: err.Body,
 	})
-	w.Write(resp)
+	if err := json.NewEncoder(w).Encode(resp); err != nil {
+		log.Error("Failed to encode error response: " + err.Error())
+	}
 }
 
 // WriteResponse writes the provided HTTPResponse instance via http.ResponseWriter
 func (hp HTTPProtocol) WriteResponse(w http.ResponseWriter, r *HTTPResponse) {
+	w.Header().Set("Content-Type", "application/json")
 	status := r.Code
-	resp, err := json.Marshal(r)
-	if err != nil {
-		status = http.StatusInternalServerError
-		resp, _ = json.Marshal(&HTTPResponse{
-			Code:    status,
-			Message: fmt.Sprintf("Error: %s", err),
-		})
-	}
-
 	w.WriteHeader(status)
-	w.Write(resp)
+	if err := json.NewEncoder(w).Encode(r); err != nil {
+		log.Error("Failed to encode response: " + err.Error())
+	}
 }

core/pkg/util/promutil/promutil.go

@@ -75,16 +75,22 @@ func LabelNamesFrom(labels map[string]string) []string {
 
 // Prepends a qualifier string to the keys provided in the m map and returns the new keys and values.
 func KubePrependQualifierToLabels(m map[string]string, qualifier string) ([]string, []string) {
-	keys := make([]string, 0, len(m))
-	for k := range m {
+	// sanitize the keys in m to prevent duplicate output keys
+	sanitizedM := make(map[string]string)
+	for k, v := range m {
+		sanitizedM[SanitizeLabelName(k)] = v
+	}
+
+	keys := make([]string, 0, len(sanitizedM))
+	for k := range sanitizedM {
 		keys = append(keys, k)
 	}
 	sort.Strings(keys)
 
-	values := make([]string, 0, len(m))
+	values := make([]string, 0, len(sanitizedM))
 	for i, k := range keys {
-		keys[i] = qualifier + SanitizeLabelName(k)
-		values = append(values, m[k])
+		keys[i] = qualifier + k
+		values = append(values, sanitizedM[k])
 	}
 
 	return keys, values

core/pkg/util/promutil/promutil_test.go

@@ -95,6 +95,53 @@ func TestKubeLabelsToPromLabels(t *testing.T) {
 	}
 }
 
+func TestKubePrependQualifierToLabelsDuplicates(t *testing.T) {
+	// 7 expected labels/values
+	expectedLabels := []string{
+		"label_app_",
+		"label_chart",
+		"label_control_plane",
+		"label_gatekeeper_sh_operation",
+		"label_heritage",
+		"label_pod_template_hash",
+		"label_release",
+	}
+	expectedValues := []string{
+		"gatekeeper",
+		"gatekeeper",
+		"audit-controller",
+		"audit",
+		"Helm",
+		"5599859cd4",
+		"gatekeeper",
+	}
+
+	// 8 input labels/values, with one duplicate label
+	kubeLabels := map[string]string{
+		// app- will be sanitized to app_
+		"app-":                    "gatekeeper",
+		"app_":                    "gatekeeper",
+		"chart":                   "gatekeeper",
+		"control-plane":           "audit-controller",
+		"gatekeeper.sh/operation": "audit",
+		"heritage":                "Helm",
+		"pod-template-hash":       "5599859cd4",
+		"release":                 "gatekeeper",
+	}
+
+	labels, values := KubePrependQualifierToLabels(kubeLabels, "label_")
+
+	// Check to make sure we get expected labels and values returned
+	err := checkSlice(labels, expectedLabels)
+	if err != nil {
+		t.Errorf("%s", err)
+	}
+	err = checkSlice(values, expectedValues)
+	if err != nil {
+		t.Errorf("%s", err)
+	}
+}
+
 func TestSanitizeLabels(t *testing.T) {
 	type testCase struct {
 		in  map[string]string

docs/testing/AUTOMATED_TESTING.md

@@ -0,0 +1,15 @@
+# Automated Testing in OpenCost
+
+This document governs OpenCost's approach to automated testing. OpenCost has two main components of automated testing, Integration Tests and Unit Tests.
+
+Unit Tests are designed to test small pieces of code. They make extensive use of mocks and other synthetic items. These tests are designed to be run quickly and easily on developers' machines. 
+
+Integration Tests are designed to test the functionality of groups of units of code working together. These are typically more complex tests that require more setup than usual. In the context of OpenCost, this means typically we are querying against a real Prometheus with real data. 
+
+## OpenCost Automation Pipeline
+
+OpenCost will execute its unit and integration tests in accordance with the following pipeline architecture:
+
+![OpenCost Test Architecture](OC%20Test%20Arch.png)
+
+The tests will execute on contributed code at different stages in the pipeline depending on whether or not the contributor of the code is a maintainer or is a third party. If the contributor is a third party, the Integration Tests will not be executed on the code until it is in the merge queue, so that unreviewed/unapproved code will not be able to execute tests or access integration test clusters. 

go.mod

@@ -8,7 +8,7 @@ replace (
 
 require (
 	cloud.google.com/go/bigquery v1.61.0
-	cloud.google.com/go/compute/metadata v0.3.0
+	cloud.google.com/go/compute/metadata v0.6.0
 	cloud.google.com/go/storage v1.42.0
 	github.com/Azure/azure-sdk-for-go v68.0.0+incompatible
 	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.17.1

go.sum

@@ -32,8 +32,8 @@ cloud.google.com/go/bigquery v1.7.0/go.mod h1://okPTzCYNXSlb24MZs83e2Do+h+VXtc4g
 cloud.google.com/go/bigquery v1.8.0/go.mod h1:J5hqkt3O0uAFnINi6JXValWIb1v0goeZM77hZzJN/fQ=
 cloud.google.com/go/bigquery v1.61.0 h1:w2Goy9n6gh91LVi6B2Sc+HpBl8WbWhIyzdvVvrAuEIw=
 cloud.google.com/go/bigquery v1.61.0/go.mod h1:PjZUje0IocbuTOdq4DBOJLNYB0WF3pAKBHzAYyxCwFo=
-cloud.google.com/go/compute/metadata v0.3.0 h1:Tz+eQXMEqDIKRsmY3cHTL6FVaynIjX2QxYC4trgAKZc=
-cloud.google.com/go/compute/metadata v0.3.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k=
+cloud.google.com/go/compute/metadata v0.6.0 h1:A6hENjEsCDtC1k8byVsgwvVcioamEHvZ4j01OwKxG9I=
+cloud.google.com/go/compute/metadata v0.6.0/go.mod h1:FjyFAW1MW0C203CEOMDTu3Dk1FlqW3Rga40jzHL4hfg=
 cloud.google.com/go/datacatalog v1.20.1 h1:czcba5mxwRM5V//jSadyig0y+8aOHmN7gUl9GbHu59E=
 cloud.google.com/go/datacatalog v1.20.1/go.mod h1:Jzc2CoHudhuZhpv78UBAjMEg3w7I9jHA11SbRshWUjk=
 cloud.google.com/go/datastore v1.0.0/go.mod h1:LXYbyblFSglQ5pkeyhO+Qmw7ukd3C+pD7TKLgZqpHYE=

justfile

@@ -46,7 +46,7 @@ build-binary VERSION=version:
         -o ./costmodel-arm64
 
 # Build and push a multi-arch Docker image
-build IMAGE_TAG RELEASE_VERSION: test (build-binary RELEASE_VERSION)
+build IMAGE_TAG RELEASE_VERSION: (build-binary RELEASE_VERSION)
     docker buildx build \
         --rm \
         --platform "linux/amd64" \

pkg/cloud/azure/provider.go

@@ -849,7 +849,16 @@ func (az *Azure) DownloadPricingData() error {
 
 	rateCardFilter := fmt.Sprintf("OfferDurableId eq '%s' and Currency eq '%s' and Locale eq 'en-US' and RegionInfo eq '%s'", config.AzureOfferDurableID, config.CurrencyCode, config.AzureBillingRegion)
 
-	log.Infof("Using ratecard query %s", rateCardFilter)
+	// create a preparer (the same way rcClient.Get() does) so that we can log the azureRateCard URL
+	log.Infof("Using azureRateCard query %s", rateCardFilter)
+	rcPreparer, err := rcClient.GetPreparer(context.TODO(), rateCardFilter)
+	if err != nil {
+		// this isn't an error that necessitates a return, as we only need the preparer for an informational log
+		log.Infof("Failed to get azureRateCard URL: %s", err)
+	} else {
+		log.Infof("Using azureRateCard URL %s", rcPreparer.URL.String())
+	}
+
 	// rate-card client is old, it can hang indefinitely in some cases
 	// this happens on the main thread, so it may block the whole app
 	// there is can be a better way to set timeout for the client
@@ -1099,10 +1108,6 @@ func (az *Azure) NodePricing(key models.Key) (*models.Node, models.PricingMetada
 
 	meta := models.PricingMetadata{}
 
-	if az.Pricing == nil {
-		return nil, meta, fmt.Errorf("Unable to download Azure pricing data")
-	}
-
 	azKey, ok := key.(*azureKey)
 	if !ok {
 		return nil, meta, fmt.Errorf("azure: NodePricing: key is of type %T", key)
@@ -1122,12 +1127,16 @@ func (az *Azure) NodePricing(key models.Key) (*models.Node, models.PricingMetada
 		featureString = azKey.Features()
 	}
 
-	if n, ok := az.Pricing[featureString]; ok {
-		log.Debugf("Returning pricing for node %s: %+v from key %s", azKey, n, azKey.Features())
-		if azKey.isValidGPUNode() {
-			n.Node.GPU = azKey.GetGPUCount()
+	if az.Pricing != nil {
+		if n, ok := az.Pricing[featureString]; ok {
+			log.Debugf("Returning pricing for node %s: %+v from key %s", azKey, n, azKey.Features())
+			if azKey.isValidGPUNode() {
+				n.Node.GPU = azKey.GetGPUCount()
+			}
+			return n.Node, meta, nil
+		} else {
+			log.Debugf("Could not find pricing for node %s from key %s", azKey, azKey.Features())
 		}
-		return n.Node, meta, nil
 	}
 
 	cost, err := getRetailPrice(region, instance, config.CurrencyCode, isSpot)

pkg/clustercache/store.go

@@ -8,7 +8,6 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
-	rt "k8s.io/apimachinery/pkg/runtime"
 )
 
 // GenericStore is a generic store implementation. It converts objects to a different type using a transform function.
@@ -87,14 +86,9 @@ func (s *GenericStore[Input, Output]) Delete(obj any) error {
 func (s *GenericStore[Input, Output]) GetAll() []Output {
 	s.mutex.RLock()
 	defer s.mutex.RUnlock()
-
-	// Deep copy the stored items to ensure that callers do not modify
-	// the original objects in the store.
 	allItems := make([]Output, 0, len(s.items))
 	for _, item := range s.items {
-		if deepCopyable, ok := any(item).(rt.Object); ok {
-			allItems = append(allItems, deepCopyable.DeepCopyObject().(Output))
-		}
+		allItems = append(allItems, item)
 	}
 	return allItems
 }

pkg/costmodel/allocation_helpers_test.go

@@ -8,13 +8,15 @@ import (
 	"github.com/opencost/opencost/core/pkg/opencost"
 	"github.com/opencost/opencost/core/pkg/source"
 	"github.com/opencost/opencost/core/pkg/util"
+	"github.com/opencost/opencost/pkg/prom"
 )
 
 const Ki = 1024
 const Mi = Ki * 1024
 const Gi = Mi * 1024
 
-const minute = 60.0
+const second = 1.0
+const minute = second * 60.0
 const hour = minute * 60.0
 
 var windowStart = time.Date(2020, 6, 16, 0, 0, 0, 0, time.UTC)
@@ -201,7 +203,6 @@ var pvMap1 = map[pvKey]*pv{
 	},
 }
 
-/* pv/pvc Helpers */
 func TestBuildPVMap(t *testing.T) {
 	pvMap1NoBytes := make(map[pvKey]*pv, len(pvMap1))
 	for thisPVKey, thisPV := range pvMap1 {
@@ -211,6 +212,9 @@ func TestBuildPVMap(t *testing.T) {
 		pvMap1NoBytes[thisPVKey] = clonePV
 	}
 
+	// These test cases are mocking behavior from Prometheus v3+
+	prometheusVersion = "3.0.0"
+
 	testCases := map[string]struct {
 		resolution              time.Duration
 		resultsPVCostPerGiBHour []*source.QueryResult
@@ -276,9 +280,6 @@ func TestBuildPVMap(t *testing.T) {
 						"persistentvolume": "pv1",
 					},
 					[]*util.Vector{
-						{
-							Timestamp: startFloat,
-						},
 						{
 							Timestamp: startFloat + (hour * 6),
 						},
@@ -297,9 +298,6 @@ func TestBuildPVMap(t *testing.T) {
 						"persistentvolume": "pv2",
 					},
 					[]*util.Vector{
-						{
-							Timestamp: startFloat,
-						},
 						{
 							Timestamp: startFloat + (hour * 6),
 						},
@@ -321,9 +319,6 @@ func TestBuildPVMap(t *testing.T) {
 						"persistentvolume": "pv3",
 					},
 					[]*util.Vector{
-						{
-							Timestamp: startFloat + (hour * 6),
-						},
 						{
 							Timestamp: startFloat + (hour * 12),
 						},
@@ -339,9 +334,6 @@ func TestBuildPVMap(t *testing.T) {
 						"persistentvolume": "pv4",
 					},
 					[]*util.Vector{
-						{
-							Timestamp: startFloat,
-						},
 						{
 							Timestamp: startFloat + (hour * 6),
 						},
@@ -384,8 +376,6 @@ func TestBuildPVMap(t *testing.T) {
 	}
 }
 
-/* Helper Helpers */
-
 func TestGetUnmountedPodForCluster(t *testing.T) {
 	testCases := map[string]struct {
 		window   opencost.Window
@@ -467,37 +457,38 @@ func TestGetUnmountedPodForCluster(t *testing.T) {
 }
 
 func TestCalculateStartAndEnd(t *testing.T) {
+	// These test cases are mocking behavior from Prometheus v3+
+	prometheusVersion = "3.0.0"
 
 	testCases := map[string]struct {
-		resolution    time.Duration
+		resolution    time.Duration   // User defined config when querying Prometheus
+		window        opencost.Window // User defined config when querying Allocations/Assets
 		expectedStart time.Time
 		expectedEnd   time.Time
 		result        *source.QueryResult
 	}{
+		// Example: avg(node_total_hourly_cost{}) by (node, provider_id)[1h:1h]
 		"1 hour resolution, 1 hour window": {
 			resolution:    time.Hour,
+			window:        opencost.NewClosedWindow(windowStart, windowStart.Add(time.Hour)),
 			expectedStart: windowStart,
 			expectedEnd:   windowStart.Add(time.Hour),
 			result: &source.QueryResult{
 				Values: []*util.Vector{
-					{
-						Timestamp: startFloat,
-					},
 					{
 						Timestamp: startFloat + (minute * 60),
 					},
 				},
 			},
 		},
+		// Example: avg(node_total_hourly_cost{}) by (node, provider_id)[1h:30m]
 		"30 minute resolution, 1 hour window": {
 			resolution:    time.Minute * 30,
+			window:        opencost.NewClosedWindow(windowStart, windowStart.Add(time.Hour)),
 			expectedStart: windowStart,
 			expectedEnd:   windowStart.Add(time.Hour),
 			result: &source.QueryResult{
 				Values: []*util.Vector{
-					{
-						Timestamp: startFloat,
-					},
 					{
 						Timestamp: startFloat + (minute * 30),
 					},
@@ -507,15 +498,14 @@ func TestCalculateStartAndEnd(t *testing.T) {
 				},
 			},
 		},
+		// Example: avg(node_total_hourly_cost{}) by (node, provider_id)[45m:15m]
 		"15 minute resolution, 45 minute window": {
 			resolution:    time.Minute * 15,
+			window:        opencost.NewClosedWindow(windowStart, windowStart.Add(time.Minute*45)),
 			expectedStart: windowStart,
 			expectedEnd:   windowStart.Add(time.Minute * 45),
 			result: &source.QueryResult{
 				Values: []*util.Vector{
-					{
-						Timestamp: startFloat + (minute * 0),
-					},
 					{
 						Timestamp: startFloat + (minute * 15),
 					},
@@ -530,13 +520,11 @@ func TestCalculateStartAndEnd(t *testing.T) {
 		},
 		"1 minute resolution, 5 minute window": {
 			resolution:    time.Minute,
+			window:        opencost.NewClosedWindow(windowStart.Add(time.Minute*15), windowStart.Add(time.Minute*20)),
 			expectedStart: windowStart.Add(time.Minute * 15),
 			expectedEnd:   windowStart.Add(time.Minute * 20),
 			result: &source.QueryResult{
 				Values: []*util.Vector{
-					{
-						Timestamp: startFloat + (minute * 15),
-					},
 					{
 						Timestamp: startFloat + (minute * 16),
 					},
@@ -555,26 +543,47 @@ func TestCalculateStartAndEnd(t *testing.T) {
 				},
 			},
 		},
+		// Example: avg(node_total_hourly_cost{}) by (node, provider_id)[5m:1m]
+		"1 minute resolution, 5 minute window, partial data": {
+			resolution:    time.Minute,
+			window:        opencost.NewClosedWindow(windowStart.Add(time.Minute*15), windowStart.Add(time.Minute*20)),
+			expectedStart: windowStart.Add(time.Minute * 18),
+			expectedEnd:   windowStart.Add(time.Minute * 20),
+			result: &prom.QueryResult{
+				Values: []*util.Vector{
+					{
+						Timestamp: startFloat + (minute * 19),
+					},
+					{
+						Timestamp: startFloat + (minute * 20),
+					},
+				},
+			},
+		},
+		// Example: avg(node_total_hourly_cost{}) by (node, provider_id)[1m:1m]
 		"1 minute resolution, 1 minute window": {
 			resolution:    time.Minute,
+			window:        opencost.NewClosedWindow(windowStart.Add(time.Minute*14).Add(time.Second*30), windowStart.Add(time.Minute*15).Add(time.Second*30)),
 			expectedStart: windowStart.Add(time.Minute * 14).Add(time.Second * 30),
 			expectedEnd:   windowStart.Add(time.Minute * 15).Add(time.Second * 30),
 			result: &source.QueryResult{
 				Values: []*util.Vector{
 					{
-						Timestamp: startFloat + (minute * 15),
+						Timestamp: startFloat + (minute * 15) + (second * 30),
 					},
 				},
 			},
 		},
+		// Example: avg(node_total_hourly_cost{}) by (node, provider_id)[1m:1m]
 		"1 minute resolution, 1 minute window, at window start": {
 			resolution:    time.Minute,
+			window:        opencost.NewClosedWindow(windowStart, windowStart.Add(time.Second*30)),
 			expectedStart: windowStart,
 			expectedEnd:   windowStart.Add(time.Second * 30),
 			result: &source.QueryResult{
 				Values: []*util.Vector{
 					{
-						Timestamp: startFloat,
+						Timestamp: startFloat + (second * 30),
 					},
 				},
 			},

pkg/costmodel/resultparsers.go

@@ -3,6 +3,8 @@ package costmodel
 import (
 	"errors"
 	"fmt"
+	"strconv"
+	"strings"
 	"time"
 
 	"github.com/opencost/opencost/core/pkg/clustercache"
@@ -12,6 +14,23 @@ import (
 	costAnalyzerCloud "github.com/opencost/opencost/pkg/cloud/models"
 )
 
+var (
+	// prometheusVersion stores the Prometheus server version (major.minor.patch).
+	// Defaults to "0.0.0" if version cannot be retrieved
+	prometheusVersion = "0.0.0"
+)
+
+// IsPrometheusVersionGTE3 returns true if the Prometheus server's major version
+// is 3 or higher.
+func IsPrometheusVersionGTE3() bool {
+	if v := strings.Split(prometheusVersion, "."); len(v) > 0 {
+		if major, err := strconv.Atoi(v[0]); err == nil && major >= 3 {
+			return true
+		}
+	}
+	return false
+}
+
 func GetPVInfoLocal(cache clustercache.ClusterCache, defaultClusterID string) (map[string]*PersistentVolumeClaimData, error) {
 	toReturn := make(map[string]*PersistentVolumeClaimData)