vtselect: fix trace search when running with a non-UTC time zone

[Vandit1604]

Fixes #260

The Jaeger and Tempo search APIs return fewer traces or none when VictoriaTraces runs with a time zone behind UTC, such as America/New_York.

findTraceIDsSplitTimeRange finds the earliest trace start by comparing strings. The _time values are UTC, but endTime was formatted in the local time zone. Behind UTC, this gave the wrong start time for the span query that follows.

The function now parses _time with logstorage.TryParseTimestampRFC3339Nano and compares Unix nanoseconds, in both copies, in query/query.go and tempo/query.go.

Same data and queries, limit=1000. Jaeger and Tempo return the same counts:

build	server TZ	1h, no filter	1h, minDuration=100ms	24h, no filter
master	UTC	534	363	1000
master	America/New_York	0	0	0
this PR	America/New_York	534	363	1000

TestSingleSearchTracesNonUTCTimezone runs vtsingle with TZ=America/New_York and checks both APIs. The apptest harness now passes environment variables to vtsingle.

The reporter of #260 runs VictoriaTraces on a host in America/Chicago, and confirmed that TZ=UTC fixes the search.

[cubic-dev-ai]

All reported issues were addressed across 3 files

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 3 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[jiekun]

Hi @Vandit1604! Thanks a lot for yet another fix. The RCA is spot‑on. That said, the string comparison (which was written by me) is clunky and not robust enough, and it also contributed to this bug.

I’d like to take this opportunity to propose changing it to a timestamp comparison instead. What do you think? You can refer to the latest revision of this branch.

[Vandit1604]

@jiekun the timestamp comparison looks good to me, and it passes the full test suite and make apptest on my side.

I also have an apptest that starts vtsingle with a non-UTC TZ and searches through the Jaeger and Tempo APIs. It fails before the fix and passes on this branch. The harness needs a small change so vtsingle gets the env var. Should I add it to this PR, or should we merge this as it is?

[jiekun]

@Vandit1604 Hi! Sure, an new test case will definitely help. Feel free to make changes to the existing vtsingle apptest setup. I will follow up and make adjustments if necessary.

[Vandit1604]

@jiekun Done. I added TestSingleSearchTracesNonUTCTimezone, which runs vtsingle with TZ=America/New_York and checks the Jaeger and Tempo search. The harness now passes env vars to vtsingle, and it has a TempoAPISearch helper in the same style as the Jaeger helpers.

I also measured your version again and updated the PR description with the new numbers.

[cubic-dev-ai]

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apptest/testcase.go">

<violation number="1" location="apptest/testcase.go:167">
P3: Merging `MustStartVtsingleWithEnv` back into `MustStartVtsingle` silently drops the documentation of the new `env` parameter: the removed helper's doc comment explicitly explained that environment variables such as `TZ` are passed to the process, while `MustStartVtsingle` (and `MustStartVtagent`) now take `env ...string` without any doc mention. Mention the optional `env` in both doc comments so tests that need a non-UTC `TZ` are discoverable.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P3: Merging MustStartVtsingleWithEnv back into MustStartVtsingle silently drops the documentation of the new env parameter: the removed helper's doc comment explicitly explained that environment variables such as TZ are passed to the process, while MustStartVtsingle (and MustStartVtagent) now take env ...string without any doc mention. Mention the optional env in both doc comments so tests that need a non-UTC TZ are discoverable.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apptest/testcase.go, line 167:

<comment>Merging `MustStartVtsingleWithEnv` back into `MustStartVtsingle` silently drops the documentation of the new `env` parameter: the removed helper's doc comment explicitly explained that environment variables such as `TZ` are passed to the process, while `MustStartVtsingle` (and `MustStartVtagent`) now take `env ...string` without any doc mention. Mention the optional `env` in both doc comments so tests that need a non-UTC `TZ` are discoverable.</comment>

<file context>
@@ -164,15 +164,7 @@ func (tc *TestCase) MustStartDefaultVtsingle() *Vtsingle {
-// MustStartVtsingleWithEnv is like MustStartVtsingle, but also passes the given
-// environment variables, such as TZ, to the vtsingle process.
-func (tc *TestCase) MustStartVtsingleWithEnv(instance string, flags, env []string) *Vtsingle {
+func (tc *TestCase) MustStartVtsingle(instance string, flags []string, env ...string) *Vtsingle {
 	tc.t.Helper()
 
</file context>

[jiekun]

Thank you for the contribution!