Skip to content

Commit 7caf035

Browse files
authored
[receiver/github] fix span times when github sends incorrect values (#44495)
#### Description Adds a corrective function to ensure timestamps that come from GitHub won't produce negative values. #### Link to tracking issue Fixes #43180
1 parent dd4a5d9 commit 7caf035

File tree

4 files changed

+132
-25
lines changed

4 files changed

+132
-25
lines changed

.chloggen/gh-span-times.yaml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog)
7+
component: receiver/github
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Adds corrections to span times when GitHub sends incorrect start and end times.
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [43180]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

receiver/githubreceiver/testdata/workflow-job-skipped-expected.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ resourceSpans:
5252
scopeSpans:
5353
- scope: {}
5454
spans:
55-
- endTimeUnixNano: "1746195354000000000"
55+
- endTimeUnixNano: "1746195355000000000"
5656
kind: 2
5757
name: build
5858
parentSpanId: 3b54c768f3ef27e1
@@ -67,11 +67,11 @@ resourceSpans:
6767
- key: cicd.pipeline.run.queue.duration
6868
value:
6969
doubleValue: 0
70-
endTimeUnixNano: "1746195354000000000"
70+
endTimeUnixNano: "1746195355000000000"
7171
kind: 2
7272
name: queue-build
7373
parentSpanId: 84e27971417e5249
7474
spanId: 750687c67fcd26a3
75-
startTimeUnixNano: "1746195354000000000"
75+
startTimeUnixNano: "1746195355000000000"
7676
status: {}
7777
traceId: 0b0cff7305ff68eb7d1bb72c7fdbe277

receiver/githubreceiver/trace_event_handling.go

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,22 @@ func newParentSpanID(runID int64, runAttempt int) (pcommon.SpanID, error) {
121121
return spanID, nil
122122
}
123123

124+
// correctActionTimestamps ensures span timestamps are valid by checking that
125+
// the end time is not before the start time. When GitHub reports timestamps
126+
// in reverse order (which can occur with skipped jobs and steps), this function
127+
// returns corrected timestamps where both start and end are set to the later
128+
// timestamp, resulting in a zero-duration span.
129+
//
130+
// This prevents negative durations that would otherwise appear as excessively
131+
// long spans in telemetry systems.
132+
func correctActionTimestamps(start, end time.Time) (time.Time, time.Time) {
133+
if end.Before(start) {
134+
// Use the later timestamp (start) for both, creating zero-duration span
135+
return start, start
136+
}
137+
return start, end
138+
}
139+
124140
// createRootSpan creates a root span based on the provided event, associated
125141
// with the deterministic traceID.
126142
func (gtr *githubTracesReceiver) createRootSpan(
@@ -141,8 +157,9 @@ func (gtr *githubTracesReceiver) createRootSpan(
141157
span.SetSpanID(rootSpanID)
142158
span.SetName(event.GetWorkflowRun().GetName())
143159
span.SetKind(ptrace.SpanKindServer)
144-
span.SetStartTimestamp(pcommon.NewTimestampFromTime(event.GetWorkflowRun().GetRunStartedAt().Time))
145-
span.SetEndTimestamp(pcommon.NewTimestampFromTime(event.GetWorkflowRun().GetUpdatedAt().Time))
160+
startTime, endTime := correctActionTimestamps(event.GetWorkflowRun().GetRunStartedAt().Time, event.GetWorkflowRun().GetUpdatedAt().Time)
161+
span.SetStartTimestamp(pcommon.NewTimestampFromTime(startTime))
162+
span.SetEndTimestamp(pcommon.NewTimestampFromTime(endTime))
146163

147164
switch strings.ToLower(event.WorkflowRun.GetConclusion()) {
148165
case "success":
@@ -207,8 +224,9 @@ func (gtr *githubTracesReceiver) createParentSpan(
207224
span.SetName(event.GetWorkflowJob().GetName())
208225
span.SetKind(ptrace.SpanKindServer)
209226

210-
span.SetStartTimestamp(pcommon.NewTimestampFromTime(event.GetWorkflowJob().GetCreatedAt().Time))
211-
span.SetEndTimestamp(pcommon.NewTimestampFromTime(event.GetWorkflowJob().GetCompletedAt().Time))
227+
startTime, endTime := correctActionTimestamps(event.GetWorkflowJob().GetCreatedAt().Time, event.GetWorkflowJob().GetCompletedAt().Time)
228+
span.SetStartTimestamp(pcommon.NewTimestampFromTime(startTime))
229+
span.SetEndTimestamp(pcommon.NewTimestampFromTime(endTime))
212230

213231
switch strings.ToLower(event.WorkflowJob.GetConclusion()) {
214232
case "success":
@@ -336,8 +354,9 @@ func (*githubTracesReceiver) createStepSpan(
336354
attrs := span.Attributes()
337355
attrs.PutStr(string(semconv.CICDPipelineTaskNameKey), name)
338356
attrs.PutStr(AttributeCICDPipelineTaskRunStatus, step.GetStatus())
339-
span.SetStartTimestamp(pcommon.NewTimestampFromTime(step.GetStartedAt().Time))
340-
span.SetEndTimestamp(pcommon.NewTimestampFromTime(step.GetCompletedAt().Time))
357+
startTime, endTime := correctActionTimestamps(step.GetStartedAt().Time, step.GetCompletedAt().Time)
358+
span.SetStartTimestamp(pcommon.NewTimestampFromTime(startTime))
359+
span.SetEndTimestamp(pcommon.NewTimestampFromTime(endTime))
341360

342361
switch strings.ToLower(step.GetConclusion()) {
343362
case "success":
@@ -404,22 +423,11 @@ func (*githubTracesReceiver) createJobQueueSpan(
404423

405424
span.SetSpanID(spanID)
406425

407-
created := pcommon.NewTimestampFromTime(event.GetWorkflowJob().GetCreatedAt().Time)
408-
started := pcommon.NewTimestampFromTime(event.GetWorkflowJob().GetStartedAt().Time)
409-
duration := event.WorkflowJob.GetStartedAt().Sub(event.GetWorkflowJob().GetCreatedAt().Time)
410-
411-
span.SetStartTimestamp(created)
412-
span.SetEndTimestamp(started)
413-
414-
// GitHub sometimes reports the createdAt value as being a second after the
415-
// startedAt value which results in unreal times in duration. To work around
416-
// this we set the duration to 0 and the start/end spans to the started
417-
// time in that event. Otherwise we calculate the time properly and set the
418-
// span start time as the created time.
419-
if created.AsTime().After(started.AsTime()) {
420-
duration = time.Duration(0)
421-
span.SetStartTimestamp(started)
422-
}
426+
createdTime, startedTime := correctActionTimestamps(event.GetWorkflowJob().GetCreatedAt().Time, event.GetWorkflowJob().GetStartedAt().Time)
427+
duration := startedTime.Sub(createdTime)
428+
429+
span.SetStartTimestamp(pcommon.NewTimestampFromTime(createdTime))
430+
span.SetEndTimestamp(pcommon.NewTimestampFromTime(startedTime))
423431

424432
attrs := span.Attributes()
425433
attrs.PutDouble(AttributeCICDPipelineRunQueueDuration, float64(duration.Nanoseconds()))

receiver/githubreceiver/trace_event_handling_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -866,3 +866,75 @@ func TestStepSpansHaveNoEvents(t *testing.T) {
866866
}
867867
}
868868
}
869+
870+
func TestCorrectActionTimestamps(t *testing.T) {
871+
tests := []struct {
872+
name string
873+
start time.Time
874+
end time.Time
875+
expectedStart time.Time
876+
expectedEnd time.Time
877+
}{
878+
{
879+
name: "normal order - no change needed",
880+
start: time.Date(2025, 5, 2, 14, 15, 54, 0, time.UTC),
881+
end: time.Date(2025, 5, 2, 14, 15, 55, 0, time.UTC),
882+
expectedStart: time.Date(2025, 5, 2, 14, 15, 54, 0, time.UTC),
883+
expectedEnd: time.Date(2025, 5, 2, 14, 15, 55, 0, time.UTC),
884+
},
885+
{
886+
name: "same timestamp - no change needed",
887+
start: time.Date(2025, 5, 2, 14, 15, 54, 0, time.UTC),
888+
end: time.Date(2025, 5, 2, 14, 15, 54, 0, time.UTC),
889+
expectedStart: time.Date(2025, 5, 2, 14, 15, 54, 0, time.UTC),
890+
expectedEnd: time.Date(2025, 5, 2, 14, 15, 54, 0, time.UTC),
891+
},
892+
{
893+
name: "inverted timestamps - end before start",
894+
start: time.Date(2025, 5, 2, 14, 15, 55, 0, time.UTC),
895+
end: time.Date(2025, 5, 2, 14, 15, 54, 0, time.UTC),
896+
expectedStart: time.Date(2025, 5, 2, 14, 15, 55, 0, time.UTC),
897+
expectedEnd: time.Date(2025, 5, 2, 14, 15, 55, 0, time.UTC),
898+
},
899+
{
900+
name: "end one second before start",
901+
start: time.Date(2025, 5, 2, 14, 15, 55, 0, time.UTC),
902+
end: time.Date(2025, 5, 2, 14, 15, 54, 0, time.UTC),
903+
expectedStart: time.Date(2025, 5, 2, 14, 15, 55, 0, time.UTC),
904+
expectedEnd: time.Date(2025, 5, 2, 14, 15, 55, 0, time.UTC),
905+
},
906+
{
907+
name: "large time difference - inverted",
908+
start: time.Date(2025, 5, 2, 15, 0, 0, 0, time.UTC),
909+
end: time.Date(2025, 5, 2, 14, 0, 0, 0, time.UTC),
910+
expectedStart: time.Date(2025, 5, 2, 15, 0, 0, 0, time.UTC),
911+
expectedEnd: time.Date(2025, 5, 2, 15, 0, 0, 0, time.UTC),
912+
},
913+
{
914+
name: "nanosecond precision - inverted",
915+
start: time.Date(2025, 5, 2, 14, 15, 55, 100, time.UTC),
916+
end: time.Date(2025, 5, 2, 14, 15, 55, 99, time.UTC),
917+
expectedStart: time.Date(2025, 5, 2, 14, 15, 55, 100, time.UTC),
918+
expectedEnd: time.Date(2025, 5, 2, 14, 15, 55, 100, time.UTC),
919+
},
920+
{
921+
name: "zero times",
922+
start: time.Time{},
923+
end: time.Time{},
924+
expectedStart: time.Time{},
925+
expectedEnd: time.Time{},
926+
},
927+
}
928+
929+
for _, tt := range tests {
930+
t.Run(tt.name, func(t *testing.T) {
931+
gotStart, gotEnd := correctActionTimestamps(tt.start, tt.end)
932+
933+
require.Equal(t, tt.expectedStart, gotStart, "start timestamp mismatch")
934+
require.Equal(t, tt.expectedEnd, gotEnd, "end timestamp mismatch")
935+
936+
// Verify the invariant: end is never before start
937+
require.False(t, gotEnd.Before(gotStart), "end timestamp should not be before start timestamp")
938+
})
939+
}
940+
}

0 commit comments

Comments
 (0)