fix: address the first point increase without previous measurement value

[srikanthccv]

Summary

In this change #6432, we introduced an expression for increase that is simplified from the increaseWithoutNegative. However, it has a case where the first value in the window doesn't have any previous value to compare to, and the counter is the cumulative value at the time, which is incorrect. This fix changes the expression, which leads to inf because we don't have a previous point. Does this mean there will be one point missing? No, because we offset by two-step intervals here

signoz/pkg/query-service/common/query_range.go

Lines 18 to 33 in c316491

	// if the query is a rate query, we adjust the start time by one more step
	// so that we can calculate the rate for the first data point
	hasRunningDiff := false
	for _, fn := range mq.Functions {
	if fn.Name == v3.FunctionNameRunningDiff {
	hasRunningDiff = true
	break
	}
	}
	if (mq.AggregateOperator.IsRateOperator() || mq.TimeAggregation.IsRateOperator()) &&
	mq.Temporality != v3.Delta {
	start -= step * 1000
	}
	if hasRunningDiff {
	start -= step * 1000
	}

so the first point is not relevant.

---

Important

Fixes experimentalIncreaseWithoutNegative in timeseries.go to handle cases without previous measurement values, preventing incorrect infinite results.

• Behavior:
  • Fixes experimentalIncreaseWithoutNegative expression in timeseries.go to handle cases without previous measurement values.
  • Adjusts calculation to prevent incorrect infinite values when the first point lacks a previous value.
• Environment:
  • Applies changes only when EXPERIMENTAL_INCREASE_WITHOUT_NEGATIVE environment variable is set.

^{This description was created by for 0a5ee65. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 0a5ee65 in 45 seconds

More details

• Looked at 22 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 5 drafted comments based on config settings.

1. pkg/query-service/app/metrics/v4/cumulative/timeseries.go:46

• Draft comment:
  The updated experimentalIncreaseWithoutNegative expression divides then multiplies by the same time difference. This seems to cancel out mathematically. Confirm that this formulation is necessary and handles edge cases correctly.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 50% <= threshold 50%
  The comment points out a potential mathematical redundancy in the code, which could be a real issue. However, it asks the author to confirm the necessity and correctness of the formulation, which violates the rule against asking for confirmation. The comment could be improved by suggesting a specific change or asking for a test to cover edge cases.

2. pkg/query-service/app/metrics/v4/cumulative/timeseries.go:170

• Draft comment:
  Passing 'start' for both %d placeholders assumes both toDateTime time boundaries are identical. Verify that using the same value is intentional.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is asking the author to verify if using the same value for both placeholders is intentional. This falls under the rule of not asking the author to confirm their intention. Therefore, this comment should be removed.

3. pkg/query-service/app/metrics/v4/cumulative/timeseries.go:46

• Draft comment:
  Consider refactoring the duplicated time-difference expression. The expression '(ts - lagInFrame(ts, 1, toDateTime(fromUnixTimestamp64Milli(%d))) OVER rate_window)' is repeated; aliasing it could improve clarity and reduce maintenance risk.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

4. pkg/query-service/app/metrics/v4/cumulative/timeseries.go:170

• Draft comment:
  Verify that using 'start' for both placeholders in fmt.Sprintf(experimentalIncreaseWithoutNegative, start, start) is intended.
• Reason this comment was not posted:
  Marked as duplicate.

5. pkg/query-service/app/metrics/v4/cumulative/timeseries.go:46

• Draft comment:
  Note that the experimental increase expression omits the 86400-second check seen in the standard increaseWithoutNegative. Ensure this behavioral change is intentional.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  This comment is asking the PR author to ensure that a behavioral change is intentional. It is not making a specific code suggestion or asking for a test to be written. Therefore, it violates the rule against asking the author to confirm their intention or ensure behavior is intended.

Workflow ID: wflow_W2u3OLs0tQwGX5fF

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.