fix review comment

Signed-off-by: googs1025 <[email protected]>

Author: googs1025

pkg/common/config.go

@@ -508,6 +508,34 @@ func (c *Configuration) validate() error {
 		if c.FakeMetrics.KVCacheUsagePercentage < 0 || c.FakeMetrics.KVCacheUsagePercentage > 1 {
 			return errors.New("fake metrics KV cache usage must be between 0 ans 1")
 		}
+		if c.FakeMetrics.RequestSuccessTotal != nil {
+			for reason, count := range c.FakeMetrics.RequestSuccessTotal {
+				if count < 0 {
+					return fmt.Errorf("fake metrics request-success-total.%s cannot be negative, got %d", reason, count)
+				}
+			}
+			requiredReasons := []string{StopFinishReason, LengthFinishReason, ToolsFinishReason, RemoteDecodeFinishReason}
+			for _, reason := range requiredReasons {
+				if _, exists := c.FakeMetrics.RequestSuccessTotal[reason]; !exists {
+					return fmt.Errorf("missing required finish reason in request-success-total: %s", reason)
+				}
+			}
+		}
+		for _, v := range c.FakeMetrics.RequestPromptTokens {
+			if v < 0 {
+				return errors.New("fake metrics request-prompt-tokens cannot contain negative values")
+			}
+		}
+		for _, v := range c.FakeMetrics.RequestGenerationTokens {
+			if v < 0 {
+				return errors.New("fake metrics request-generation-tokens cannot contain negative values")
+			}
+		}
+		for _, v := range c.FakeMetrics.RequestParamsMaxTokens {
+			if v < 0 {
+				return errors.New("fake metrics request-params-max-tokens cannot contain negative values")
+			}
+		}
 	}
 
 	if c.DPSize < 1 || c.DPSize > 8 {

pkg/llm-d-inference-sim/metrics.go

@@ -194,6 +194,7 @@ func (s *VllmSimulator) setInitialPrometheusMetrics() {
 		nRunningReqs = float64(s.config.FakeMetrics.RunningRequests)
 		nWaitingReqs = float64(s.config.FakeMetrics.WaitingRequests)
 		kvCacheUsage = float64(s.config.FakeMetrics.KVCacheUsagePercentage)
+<<<<<<< HEAD
 
 		if s.config.FakeMetrics.TTFTBucketValues != nil {
 			s.initFakeHistogram(s.ttft, common.TTFTBucketsBoundaries, s.config.FakeMetrics.TTFTBucketValues)
@@ -211,9 +212,23 @@ func (s *VllmSimulator) setInitialPrometheusMetrics() {
 		for _, requestParamsMaxToken := range s.config.FakeMetrics.RequestParamsMaxTokens {
 			s.requestParamsMaxTokens.WithLabelValues(modelName).Observe(requestParamsMaxToken)
 		}
+=======
+>>>>>>> 02bc5c3 (fix review comment)
 		for reason, requestSuccessTotal := range s.config.FakeMetrics.RequestSuccessTotal {
 			s.requestSuccessTotal.WithLabelValues(modelName, reason).Add(float64(requestSuccessTotal))
 		}
+		buckets := build125Buckets(s.config.MaxModelLen)
+		for _, sample := range generateSamplesFromBuckets(buckets, s.config.FakeMetrics.RequestPromptTokens) {
+			s.requestPromptTokens.WithLabelValues(modelName).Observe(sample)
+		}
+
+		for _, sample := range generateSamplesFromBuckets(buckets, s.config.FakeMetrics.RequestGenerationTokens) {
+			s.requestGenerationTokens.WithLabelValues(modelName).Observe(sample)
+		}
+
+		for _, sample := range generateSamplesFromBuckets(buckets, s.config.FakeMetrics.RequestParamsMaxTokens) {
+			s.requestParamsMaxTokens.WithLabelValues(modelName).Observe(sample)
+		}
 
 	}
 
@@ -539,3 +554,55 @@ func build125Buckets(maxValue int) []float64 {
 	}
 	return buckets
 }
+
+// padCountsToFull pads the counts slice to length len(boundaries)+1 by appending zeros.
+func padCountsToFull(boundaries []float64, counts []float64) []float64 {
+	fullLen := len(boundaries) + 1
+	if len(counts) > fullLen {
+		return counts[:fullLen] // just return limit len
+	}
+	padded := make([]float64, fullLen)
+	copy(padded, counts)
+	// rest are zero by default
+	return padded
+}
+
+func generateSamplesFromBuckets(boundaries []float64, counts []float64) []float64 {
+	fullCounts := padCountsToFull(boundaries, counts)
+	var samples []float64
+
+	for i, count := range fullCounts {
+		if count == 0 {
+			continue
+		}
+
+		var val float64
+		if len(boundaries) == 0 {
+			// No boundaries → one bucket (-Inf, +Inf)
+			val = 1.0
+		} else if i == 0 {
+			// Bucket: (-Inf, boundaries[0]]
+			val = boundaries[0] - 1.0
+			if val <= 0 { // avoid non-positive if boundary is small
+				val = boundaries[0] * 0.5
+			}
+		} else if i < len(boundaries) {
+			// Bucket: (boundaries[i-1], boundaries[i]]
+			lower := boundaries[i-1]
+			upper := boundaries[i]
+			val = (lower + upper) / 2.0
+			// Ensure it's strictly > lower and <= upper
+			if val <= lower {
+				val = upper - (upper-lower)*0.1
+			}
+		} else {
+			// Last bucket: (boundaries[len-1], +Inf)
+			val = boundaries[len(boundaries)-1] + 1.0
+		}
+
+		for j := 0; j < int(count); j++ {
+			samples = append(samples, val)
+		}
+	}
+	return samples
+}

pkg/llm-d-inference-sim/metrics_test.go

@@ -19,7 +19,9 @@ package llmdinferencesim
 import (
 	"context"
 	"errors"
+	"fmt"
 	"io"
+	"math"
 	"net/http"
 	"os"
 	"reflect"
@@ -109,7 +111,7 @@ var _ = Describe("Simulator metrics", Ordered, func() {
 
 	It("Should record correct prompt and generation token counts", func() {
 		modelName := "testmodel"
-		prompt := strings.Repeat("hello ", 10)
+		prompt := strings.Repeat("hello ", 25)
 		maxTokens := 25
 
 		ctx := context.TODO()
@@ -144,10 +146,38 @@ var _ = Describe("Simulator metrics", Ordered, func() {
 		data, err := io.ReadAll(metricsResp.Body)
 		Expect(err).NotTo(HaveOccurred())
 		metrics := string(data)
+		// request_prompt_tokens_bucket
+		Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="testmodel",le="1"} 0`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="testmodel",le="2"} 0`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="testmodel",le="5"} 0`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="testmodel",le="10"} 0`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="testmodel",le="20"} 0`))
 		Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="testmodel",le="50"} 1`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="testmodel",le="100"} 1`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="testmodel",le="200"} 1`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="testmodel",le="500"} 1`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="testmodel",le="100"} 1`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="testmodel",le="+Inf"} 1`))
+		// request_params_max_tokens_bucket
+		Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="testmodel",le="1"} 0`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="testmodel",le="2"} 0`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="testmodel",le="5"} 0`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="testmodel",le="10"} 0`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="testmodel",le="20"} 0`))
 		Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="testmodel",le="50"} 1`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="testmodel",le="100"} 1`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="testmodel",le="200"} 1`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="testmodel",le="500"} 1`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="testmodel",le="1000"} 1`))
+		Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="testmodel",le="+Inf"} 1`))
+		// request_generation_tokens
+		// We do not verify the distribution of the number of tokens generated per request,
+		// as the number of generated tokens is unpredictable in this test.
+		// Therefore, we only verify the number of requests and the total number of generated tokens,
+		// and skip the bucket distribution.
 		Expect(metrics).To(ContainSubstring(`vllm:request_generation_tokens_count{model_name="testmodel"} 1`))
-		Expect(metrics).To(ContainSubstring(`vllm:request_success_total{finish_reason="stop",model_name="testmodel"} 1`))
+		// request_success_total
+		Expect(metrics).To(MatchRegexp(`vllm:request_success_total{finish_reason="(stop|length)",model_name="testmodel"} 1`))
 	})
 
 	It("Should send correct lora metrics", func() {
@@ -632,7 +662,36 @@ var _ = Describe("Simulator metrics", Ordered, func() {
 			ctx := context.TODO()
 			args := []string{"cmd", "--model", model, "--mode", common.ModeRandom,
 				"--fake-metrics",
+<<<<<<< HEAD
 				"{\"running-requests\":10,\"waiting-requests\":30,\"kv-cache-usage\":0.4,\"loras\":[{\"running\":\"lora4,lora2\",\"waiting\":\"lora3\",\"timestamp\":1257894567},{\"running\":\"lora4,lora3\",\"waiting\":\"\",\"timestamp\":1257894569}],\"ttft-buckets-values\":[1, 2, 3],\"tpot-buckets-values\": [0, 0, 1, 2, 3]}",
+=======
+				`{` +
+					`"running-requests":10,` +
+					`"waiting-requests":30,` +
+					`"kv-cache-usage":0.4,` +
+					`"request-success-total":{` +
+					`"stop":20,` +
+					`"length":0,` +
+					`"tool_calls":0,` +
+					`"remote_decode":0` +
+					`},` +
+					`"request-prompt-tokens":[10,20,30],` +
+					`"request-generation-tokens":[10,20,30],` +
+					`"request-params-max-tokens":[10,20,30],` +
+					`"loras":[` +
+					`{` +
+					`"running":"lora4,lora2",` +
+					`"waiting":"lora3",` +
+					`"timestamp":1257894567` +
+					`},` +
+					`{` +
+					`"running":"lora4,lora3",` +
+					`"waiting":"",` +
+					`"timestamp":1257894569` +
+					`}` +
+					`]` +
+					`}`,
+>>>>>>> 02bc5c3 (fix review comment)
 			}
 
 			client, err := startServerWithArgs(ctx, common.ModeRandom, args, nil)
@@ -651,6 +710,7 @@ var _ = Describe("Simulator metrics", Ordered, func() {
 			Expect(metrics).To(ContainSubstring("vllm:lora_requests_info{max_lora=\"1\",running_lora_adapters=\"lora4,lora2\",waiting_lora_adapters=\"lora3\"} 1.257894567e+09"))
 			Expect(metrics).To(ContainSubstring("vllm:lora_requests_info{max_lora=\"1\",running_lora_adapters=\"lora4,lora3\",waiting_lora_adapters=\"\"} 1.257894569e+09"))
 
+<<<<<<< HEAD
 			Expect(metrics).To(ContainSubstring("vllm:time_to_first_token_seconds_bucket{model_name=\"my_model\",le=\"0.001\"} 1"))
 			Expect(metrics).To(ContainSubstring("vllm:time_to_first_token_seconds_bucket{model_name=\"my_model\",le=\"0.005\"} 3"))
 			Expect(metrics).To(ContainSubstring("vllm:time_to_first_token_seconds_bucket{model_name=\"my_model\",le=\"0.01\"} 6"))
@@ -707,6 +767,49 @@ var _ = Describe("Simulator metrics", Ordered, func() {
 			Expect(metrics).To(ContainSubstring("vllm:time_to_first_token_seconds_bucket{model_name=\"my_model\",le=\"640\"} 0"))
 			Expect(metrics).To(ContainSubstring("vllm:time_to_first_token_seconds_bucket{model_name=\"my_model\",le=\"2560\"} 0"))
 			Expect(metrics).To(ContainSubstring("vllm:time_to_first_token_seconds_bucket{model_name=\"my_model\",le=\"+Inf\"} 1"))
+=======
+			Expect(metrics).To(ContainSubstring(`vllm:request_generation_tokens_bucket{model_name="my_model",le="1"} 10`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_generation_tokens_bucket{model_name="my_model",le="2"} 30`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_generation_tokens_bucket{model_name="my_model",le="5"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_generation_tokens_bucket{model_name="my_model",le="10"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_generation_tokens_bucket{model_name="my_model",le="20"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_generation_tokens_bucket{model_name="my_model",le="50"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_generation_tokens_bucket{model_name="my_model",le="100"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_generation_tokens_bucket{model_name="my_model",le="200"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_generation_tokens_bucket{model_name="my_model",le="500"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_generation_tokens_bucket{model_name="my_model",le="1000"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_generation_tokens_bucket{model_name="my_model",le="+Inf"} 60`))
+
+			Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="my_model",le="1"} 10`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="my_model",le="2"} 30`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="my_model",le="5"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="my_model",le="10"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="my_model",le="20"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="my_model",le="50"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="my_model",le="100"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="my_model",le="200"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="my_model",le="500"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="my_model",le="1000"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_prompt_tokens_bucket{model_name="my_model",le="+Inf"} 60`))
+
+			Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="my_model",le="1"} 10`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="my_model",le="2"} 30`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="my_model",le="5"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="my_model",le="10"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="my_model",le="20"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="my_model",le="50"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="my_model",le="100"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="my_model",le="200"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="my_model",le="500"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="my_model",le="1000"} 60`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_params_max_tokens_bucket{model_name="my_model",le="+Inf"} 60`))
+
+			Expect(metrics).To(ContainSubstring(`vllm:request_success_total{finish_reason="length",model_name="my_model"} 0`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_success_total{finish_reason="remote_decode",model_name="my_model"} 0`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_success_total{finish_reason="stop",model_name="my_model"} 20`))
+			Expect(metrics).To(ContainSubstring(`vllm:request_success_total{finish_reason="tool_calls",model_name="my_model"} 0`))
+
+>>>>>>> 02bc5c3 (fix review comment)
 		})
 	})
 })
@@ -890,3 +993,104 @@ func TestBuild125Buckets(t *testing.T) {
 		})
 	}
 }
+
+func validateSamplesInBuckets(t *testing.T, samples []float64, boundaries []float64, counts []float64) {
+	fullCounts := padCountsToFull(boundaries, counts)
+	// Now validate using fullCounts
+	sortedSamples := make([]float64, len(samples))
+	copy(sortedSamples, samples)
+	sort.Float64s(sortedSamples)
+
+	actualCounts := make([]int, len(fullCounts))
+	sampleIndex := 0
+
+	for bucketIndex := range fullCounts {
+		var upperBound float64
+		if bucketIndex == len(boundaries) {
+			upperBound = math.Inf(+1)
+		} else {
+			upperBound = boundaries[bucketIndex]
+		}
+
+		for sampleIndex < len(sortedSamples) && sortedSamples[sampleIndex] <= upperBound {
+			actualCounts[bucketIndex]++
+			sampleIndex++
+		}
+	}
+
+	// Verify each bucket
+	for i, want := range fullCounts {
+		if actualCounts[i] != int(want) {
+			var lowerStr, upperStr string
+			if i == 0 {
+				lowerStr = "-Inf"
+			} else {
+				lowerStr = fmt.Sprintf("%.3f", boundaries[i-1])
+			}
+			if i == len(boundaries) {
+				upperStr = "+Inf"
+			} else {
+				upperStr = fmt.Sprintf("%.3f", boundaries[i])
+			}
+			t.Errorf("bucket[%d] (%s, %s]: want %d, got %d",
+				i, lowerStr, upperStr, int(want), actualCounts[i])
+		}
+	}
+
+	totalExpected := 0
+	for _, c := range fullCounts {
+		totalExpected += int(c)
+	}
+	if len(samples) != totalExpected {
+		t.Errorf("total samples: want %d, got %d", totalExpected, len(samples))
+	}
+}
+
+func TestGenerateSamplesFromBuckets(t *testing.T) {
+	tests := []struct {
+		name            string
+		boundaries      []float64
+		counts          []float64
+		expectedSamples int
+	}{
+		{
+			name:            "short counts with non-zero in middle",
+			boundaries:      []float64{1, 2, 5, 10, 20, 50, 100, 200, 500, 1000},
+			counts:          []float64{0, 0, 0, 5, 0, 5},
+			expectedSamples: 10,
+		},
+		{
+			name:            "empty boundaries → 1 bucket",
+			boundaries:      []float64{},
+			counts:          []float64{7},
+			expectedSamples: 7,
+		},
+		{
+			name:            "single boundary → 2 buckets, short counts",
+			boundaries:      []float64{10.0},
+			counts:          []float64{3},
+			expectedSamples: 3,
+		},
+		{
+			name:            "full counts provided",
+			boundaries:      []float64{1, 2, 5},
+			counts:          []float64{1, 0, 2, 1},
+			expectedSamples: 4,
+		},
+		{
+			name:            "all zeros (short)",
+			boundaries:      []float64{1, 2, 5},
+			counts:          []float64{},
+			expectedSamples: 0,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			samples := generateSamplesFromBuckets(tt.boundaries, tt.counts)
+			if len(samples) != tt.expectedSamples {
+				t.Fatalf("sample count mismatch: want %d, got %d", tt.expectedSamples, len(samples))
+			}
+			validateSamplesInBuckets(t, samples, tt.boundaries, tt.counts)
+		})
+	}
+}