Skip to content

Commit f179c95

Browse files
harp-intelharp-intel
committed
Merge branch 'main' into fiojob
2 parents eace668 + 9986904 commit f179c95

File tree

7 files changed

+210
-63
lines changed

7 files changed

+210
-63
lines changed

‎cmd/metrics/metadata.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ type Metadata struct {
7272
// below are not loaded by LoadMetadata, but are set by the caller (should these be here at all?)
7373
CollectionStartTime time.Time
7474
PerfSpectVersion string
75+
WithWorkload bool // true if metrics were collected with a user-provided workload application
7576
}
7677

7778
// LoadMetadata - populates and returns a Metadata structure containing state of the

‎cmd/metrics/metrics.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -833,13 +833,29 @@ func processRawData(localOutputDir string) error {
833833
printOutputFileNames([][]string{filesWritten})
834834
return nil
835835
}
836+
837+
func needsOutputDir(cmd *cobra.Command) bool {
838+
return !flagLive && !flagPrometheusServer && !cmd.Flags().Changed("prometheus-server-addr") && !flagShowMetricNames
839+
}
840+
836841
func runCmd(cmd *cobra.Command, args []string) error {
837842
// appContext is the application context that holds common data and resources.
838843
appContext := cmd.Parent().Context().Value(common.AppContext{}).(common.AppContext)
839844
localTempDir := appContext.LocalTempDir
840845
localOutputDir := appContext.OutputDir
841846
// Setup signal manager for coordinated shutdown
842847
signalMgr := newSignalManager()
848+
// create output directory if needed
849+
if needsOutputDir(cmd) {
850+
err := util.CreateDirectoryIfNotExists(localOutputDir, 0755) // #nosec G301
851+
if err != nil {
852+
err = fmt.Errorf("failed to create output directory: %w", err)
853+
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
854+
slog.Error(err.Error())
855+
cmd.SilenceUsage = true
856+
return err
857+
}
858+
}
843859
// short circuit when --input flag is set
844860
if flagInput != "" {
845861
// skip data collection and use raw data for reports
@@ -1320,6 +1336,7 @@ func collectOnTarget(targetContext *targetContext, localTempDir string, localOut
13201336
printCompleteChannel := make(chan []string)
13211337
// get current time for use in setting timestamps on output
13221338
targetContext.metadata.CollectionStartTime = time.Now() // save the start time in the metadata for use when using the --input option to process raw data
1339+
targetContext.metadata.WithWorkload = len(argsApplication) > 0
13231340
go printMetricsAsync(targetContext, localOutputDir, frameChannel, printCompleteChannel)
13241341
var err error
13251342
for !signalMgr.shouldStop() {

‎cmd/metrics/summary.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ func summarizeMetrics(localOutputDir string, targetName string, metadata Metadat
3333
if err != nil {
3434
return filesCreated, fmt.Errorf("failed to read metrics from %s: %w", csvMetricsFile, err)
3535
}
36+
// exclude the final sample if metrics were collected with a workload
37+
if metadata.WithWorkload {
38+
metrics.excludeFinalSample()
39+
}
3640
// csv summary
3741
out, err := metrics.getCSV()
3842
if err != nil {
@@ -214,6 +218,40 @@ func newMetricCollection(csvPath string) (metrics MetricCollection, err error) {
214218
return
215219
}
216220

221+
// excludeFinalSample removes the final timestamp's rows from all metric groups.
222+
// This is used when collecting metrics with a workload to avoid including
223+
// post-workload data that can skew the summary statistics.
224+
func (mc MetricCollection) excludeFinalSample() {
225+
if len(mc) == 0 {
226+
return
227+
}
228+
// All metric groups should have the same number of rows since they come from the same CSV
229+
// Check the first group to avoid redundant checking
230+
if len(mc[0].rows) <= 1 {
231+
// Don't exclude if there's only one sample or no samples
232+
slog.Warn("metric collection has only one sample, not excluding final sample")
233+
return
234+
}
235+
for i := range mc {
236+
// Find the maximum timestamp in this group
237+
maxTimestamp := mc[i].rows[0].timestamp
238+
for _, row := range mc[i].rows {
239+
if row.timestamp > maxTimestamp {
240+
maxTimestamp = row.timestamp
241+
}
242+
}
243+
// Remove all rows with the maximum timestamp
244+
var filteredRows []row
245+
for _, row := range mc[i].rows {
246+
if row.timestamp != maxTimestamp {
247+
filteredRows = append(filteredRows, row)
248+
}
249+
}
250+
mc[i].rows = filteredRows
251+
}
252+
slog.Debug("excluded final sample from metric collection", slog.Int("num_groups", len(mc)))
253+
}
254+
217255
// getStats - calculate summary stats (min, max, mean, stddev) for each metric
218256
func (mg *MetricGroup) getStats() (stats map[string]metricStats, err error) {
219257
stats = make(map[string]metricStats)

‎cmd/metrics/summary_test.go‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
package metrics
2+
3+
// Copyright (C) 2021-2025 Intel Corporation
4+
// SPDX-License-Identifier: BSD-3-Clause
5+
6+
import (
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
func TestExcludeFinalSample(t *testing.T) {
13+
tests := []struct {
14+
name string
15+
inputRows []row
16+
expectedCount int
17+
expectedMaxTS float64
18+
}{
19+
{
20+
name: "exclude single final timestamp",
21+
inputRows: []row{
22+
{timestamp: 5.0, metrics: map[string]float64{"metric1": 100.0}},
23+
{timestamp: 10.0, metrics: map[string]float64{"metric1": 200.0}},
24+
{timestamp: 15.0, metrics: map[string]float64{"metric1": 150.0}},
25+
{timestamp: 20.0, metrics: map[string]float64{"metric1": 50.0}}, // this should be excluded
26+
},
27+
expectedCount: 3,
28+
expectedMaxTS: 15.0,
29+
},
30+
{
31+
name: "exclude multiple rows with same final timestamp",
32+
inputRows: []row{
33+
{timestamp: 5.0, socket: "0", metrics: map[string]float64{"metric1": 100.0}},
34+
{timestamp: 10.0, socket: "0", metrics: map[string]float64{"metric1": 200.0}},
35+
{timestamp: 15.0, socket: "0", metrics: map[string]float64{"metric1": 150.0}},
36+
{timestamp: 15.0, socket: "1", metrics: map[string]float64{"metric1": 160.0}}, // same timestamp, different socket
37+
},
38+
expectedCount: 2,
39+
expectedMaxTS: 10.0,
40+
},
41+
{
42+
name: "single sample - should not exclude",
43+
inputRows: []row{
44+
{timestamp: 5.0, metrics: map[string]float64{"metric1": 100.0}},
45+
},
46+
expectedCount: 1,
47+
expectedMaxTS: 5.0,
48+
},
49+
{
50+
name: "two samples - exclude last one",
51+
inputRows: []row{
52+
{timestamp: 5.0, metrics: map[string]float64{"metric1": 100.0}},
53+
{timestamp: 10.0, metrics: map[string]float64{"metric1": 50.0}},
54+
},
55+
expectedCount: 1,
56+
expectedMaxTS: 5.0,
57+
},
58+
}
59+
60+
for _, tt := range tests {
61+
t.Run(tt.name, func(t *testing.T) {
62+
// Create a MetricCollection with a single MetricGroup
63+
mc := MetricCollection{
64+
MetricGroup{
65+
names: []string{"metric1"},
66+
rows: tt.inputRows,
67+
groupByField: "",
68+
groupByValue: "",
69+
},
70+
}
71+
72+
// Call excludeFinalSample
73+
mc.excludeFinalSample()
74+
75+
// Verify the number of remaining rows
76+
assert.Equal(t, tt.expectedCount, len(mc[0].rows), "unexpected number of rows after exclusion")
77+
78+
// Verify that no row has a timestamp greater than expectedMaxTS
79+
if len(mc[0].rows) > 0 {
80+
for _, row := range mc[0].rows {
81+
assert.LessOrEqual(t, row.timestamp, tt.expectedMaxTS, "found row with timestamp greater than expected maximum")
82+
}
83+
}
84+
})
85+
}
86+
}
87+
88+
func TestExcludeFinalSampleMultipleGroups(t *testing.T) {
89+
// Test with multiple metric groups (e.g., multiple sockets)
90+
mc := MetricCollection{
91+
MetricGroup{
92+
names: []string{"metric1"},
93+
groupByField: "SKT",
94+
groupByValue: "0",
95+
rows: []row{
96+
{timestamp: 5.0, socket: "0", metrics: map[string]float64{"metric1": 100.0}},
97+
{timestamp: 10.0, socket: "0", metrics: map[string]float64{"metric1": 200.0}},
98+
{timestamp: 15.0, socket: "0", metrics: map[string]float64{"metric1": 50.0}}, // should be excluded
99+
},
100+
},
101+
MetricGroup{
102+
names: []string{"metric1"},
103+
groupByField: "SKT",
104+
groupByValue: "1",
105+
rows: []row{
106+
{timestamp: 5.0, socket: "1", metrics: map[string]float64{"metric1": 110.0}},
107+
{timestamp: 10.0, socket: "1", metrics: map[string]float64{"metric1": 210.0}},
108+
{timestamp: 15.0, socket: "1", metrics: map[string]float64{"metric1": 60.0}}, // should be excluded
109+
},
110+
},
111+
}
112+
113+
mc.excludeFinalSample()
114+
115+
// Both groups should have 2 rows remaining
116+
assert.Equal(t, 2, len(mc[0].rows), "socket 0 should have 2 rows")
117+
assert.Equal(t, 2, len(mc[1].rows), "socket 1 should have 2 rows")
118+
119+
// Verify max timestamps
120+
assert.Equal(t, 10.0, mc[0].rows[1].timestamp, "socket 0 max timestamp should be 10.0")
121+
assert.Equal(t, 10.0, mc[1].rows[1].timestamp, "socket 1 max timestamp should be 10.0")
122+
}
123+
124+
func TestExcludeFinalSampleEmptyCollection(t *testing.T) {
125+
// Test with empty MetricCollection
126+
mc := MetricCollection{}
127+
mc.excludeFinalSample() // should not panic
128+
assert.Equal(t, 0, len(mc), "empty collection should remain empty")
129+
}
130+
131+
func TestExcludeFinalSampleEmptyRows(t *testing.T) {
132+
// Test with MetricGroup that has no rows
133+
mc := MetricCollection{
134+
MetricGroup{
135+
names: []string{"metric1"},
136+
groupByField: "",
137+
groupByValue: "",
138+
rows: []row{},
139+
},
140+
}
141+
mc.excludeFinalSample() // should not panic
142+
assert.Equal(t, 0, len(mc[0].rows), "empty rows should remain empty")
143+
}

‎cmd/root.go‎

Lines changed: 0 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -211,43 +211,6 @@ func initializeApplication(cmd *cobra.Command, args []string) error {
211211
if gLogFile != nil {
212212
logFilePath = gLogFile.Name()
213213
}
214-
// create the output directory now to fail fast if there are permission or disk space issues
215-
// this validates write access before any data collection begins
216-
// skip creating output directory for config command since it doesn't generate output files
217-
// also skip for metrics command with --live, --prometheus-server, or --list flags since they don't write files
218-
// also skip for update command since it doesn't generate output files
219-
needsOutputDir := true
220-
if cmd.Name() == "config" || cmd.Name() == "update" {
221-
needsOutputDir = false
222-
} else if cmd.Name() == "metrics" {
223-
// check if --live flag is set
224-
if liveFlag, err := cmd.Flags().GetBool("live"); err == nil && liveFlag {
225-
needsOutputDir = false
226-
}
227-
// check if --prometheus-server flag is set
228-
if prometheusFlag, err := cmd.Flags().GetBool("prometheus-server"); err == nil && prometheusFlag {
229-
needsOutputDir = false
230-
}
231-
// check if --prometheus-server-addr flag has been changed (which implies prometheus server mode)
232-
if cmd.Flags().Changed("prometheus-server-addr") {
233-
needsOutputDir = false
234-
}
235-
// check if --list flag is set (just lists metrics and exits)
236-
if listFlag, err := cmd.Flags().GetBool("list"); err == nil && listFlag {
237-
needsOutputDir = false
238-
}
239-
}
240-
if needsOutputDir {
241-
created, err := createOutputDir(outputDir)
242-
if err != nil {
243-
slog.Error("failed to create output directory", slog.String("path", outputDir), slog.String("error", err.Error()))
244-
fmt.Printf("Error: failed to create output directory: %v\n", err)
245-
os.Exit(1)
246-
}
247-
if created {
248-
slog.Debug("output directory created", slog.String("path", outputDir))
249-
}
250-
}
251214
// set app context
252215
cmd.Parent().SetContext(
253216
context.WithValue(
@@ -350,30 +313,6 @@ func onIntelNetwork() bool {
350313
return true
351314
}
352315

353-
// createOutputDir creates the output directory if it does not exist.
354-
// Returns true if the directory was created, false if it already existed.
355-
func createOutputDir(outputDir string) (bool, error) {
356-
// Check if directory already exists
357-
info, err := os.Stat(outputDir)
358-
if err == nil {
359-
// Path exists, verify it's a directory
360-
if !info.IsDir() {
361-
return false, fmt.Errorf("output path exists but is not a directory: %s", outputDir)
362-
}
363-
return false, nil // Already exists
364-
}
365-
// If error is not "not exists", something else is wrong
366-
if !os.IsNotExist(err) {
367-
return false, fmt.Errorf("failed to check output directory: %w", err)
368-
}
369-
// Directory doesn't exist, create it
370-
err = os.MkdirAll(outputDir, 0755) // #nosec G301
371-
if err != nil {
372-
return false, fmt.Errorf("failed to create output directory: %w", err)
373-
}
374-
return true, nil // Created successfully
375-
}
376-
377316
func checkForUpdates(version string) (bool, manifest, error) {
378317
latestManifest, err := getLatestManifest()
379318
if err != nil {

‎internal/common/common.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,15 @@ func (rc *ReportingCommand) Run() error {
125125
slog.Error("error sending signal to children", slog.String("error", err.Error()))
126126
}
127127
}()
128+
// create output directory
129+
err := util.CreateDirectoryIfNotExists(outputDir, 0755) // #nosec G301
130+
if err != nil {
131+
err = fmt.Errorf("failed to create output directory: %w", err)
132+
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
133+
slog.Error(err.Error())
134+
rc.Cmd.SilenceUsage = true
135+
return err
136+
}
128137

129138
var orderedTargetScriptOutputs []TargetScriptOutputs
130139
var myTargets []target.Target
@@ -206,7 +215,7 @@ func (rc *ReportingCommand) Run() error {
206215
}
207216
// create the raw report before processing the data, so that we can save the raw data even if there is an error while processing
208217
var rawReports []string
209-
rawReports, err := rc.createRawReports(appContext, orderedTargetScriptOutputs)
218+
rawReports, err = rc.createRawReports(appContext, orderedTargetScriptOutputs)
210219
if err != nil {
211220
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
212221
slog.Error(err.Error())

‎tools/build.Dockerfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ RUN git clone https://github.com/madler/zlib.git zlib-aarch64 \
7575
RUN mkdir workdir
7676
ADD . /workdir
7777
WORKDIR /workdir
78-
RUN make tools
78+
RUN make tools -j$(nproc)
7979
RUN make oss-source
8080

8181
FROM ubuntu:22.04 AS perf-builder

0 commit comments

Comments
 (0)