[receiver/github] add workflow job trace events #38016
Conversation
adrielp
requested review from
andrzej-stencel,
crobert-1,
TylerHelmuth and
a team
as code owners
| attrs := resource.Attributes() | ||
| var err error | ||
|
|
||
| svc, err := gtr.getServiceName(e.GetRepo().CustomProperties["service_name"], e.GetRepo().GetName()) |
There was a problem hiding this comment.
I see this is following the logic of existing methods, so I apologize for potentially rehashing this: Is there any possibility that e is nil here? Or any sub-call from it? If so, should we be checking?
| } | ||
|
|
||
| // newUniqueSteps creates a new slice of step names from the provided GitHub | ||
| // event steps. Each step name, if duplicated, is appended with `_n` where n is |
There was a problem hiding this comment.
| // event steps. Each step name, if duplicated, is appended with `_n` where n is | |
| // event steps. Each step name, if duplicated, is appended with `-n` where n is |
Nit, looks like it's actually a dash rather than underscore. (I don't have a strong preference which it should be.)
| attrs.PutStr(AttributeCICDPipelineTaskRunStatus, AttributeCICDPipelineTaskRunStatusFailure) | ||
| span.Status().SetCode(ptrace.StatusCodeError) | ||
| case "skipped": | ||
| attrs.PutStr(AttributeCICDPipelineTaskRunStatus, AttributeCICDPipelineTaskRunStatusFailure) |
There was a problem hiding this comment.
| attrs.PutStr(AttributeCICDPipelineTaskRunStatus, AttributeCICDPipelineTaskRunStatusFailure) | |
| attrs.PutStr(AttributeCICDPipelineTaskRunStatus, AttributeCICDPipelineTaskRunStatusSkip) |
Unless I'm missing something
| attrs.PutStr(AttributeCICDPipelineTaskRunSenderLogin, e.GetSender().GetLogin()) | ||
| attrs.PutStr(semconv.AttributeCicdPipelineTaskRunURLFull, e.GetWorkflowJob().GetHTMLURL()) | ||
| attrs.PutInt(semconv.AttributeCicdPipelineTaskRunID, e.GetWorkflowJob().GetID()) | ||
| switch status := strings.ToLower(e.GetWorkflowJob().GetConclusion()); status { |
There was a problem hiding this comment.
Sanity check: We have an attribute declared as AttributeCICDPipelineTaskRunStatusError, that currently isn't used. Is it relevant in this switch?
Same question for the other switch in createStepSpan
If it's not relevant, when would it be? As you point out in the list of valid values, error isn't listed. Is it necessary?
Description
Adds workflow job trace event support, creating deterministic Span IDs throughout.
Link to tracking issue
Fixes
Testing
Unit tests & examining behavior against in prod running version.
Documentation
Updated README which contains additional information on resources and deterministic IDs.