-
Notifications
You must be signed in to change notification settings - Fork 651
Add a resource detector for Gitlab CICD #6760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
01f4338
to
abec055
Compare
abec055
to
2eddc5a
Compare
detectors/cicd/gitlab/gitlab.go
Outdated
|
||
ciPipelineName := os.Getenv(gitlabPipelineNameEnvVar) | ||
if ciPipelineName != "" { | ||
attributes = append(attributes, attribute.String(string(semconv.CICDPipelineNameKey), ciPipelineName)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can adjust this kind of format. semconv.CICDPipelineNameKey.String("xxxxx")
detectors/cicd/gitlab/gitlab_test.go
Outdated
|
||
func setTestEnv(t *testing.T, envs []EnvPair) { | ||
for _, env := range envs { | ||
err := os.Setenv(env.Key, env.Value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
os.Setenv
be changed to t.Setenv
We might need to add a configuration here: versions.yaml Of course, the specific location to add it might need to be evaluated by @open-telemetry/go-approvers. Don't forget there are two similar files: |
added as experimental version, but can't find version for the another component https://github.com/open-telemetry/opentelemetry-go-contrib/tree/main/detectors/azure/azurevm. not sure if this is expected for experimental versions |
Did you go through our new instrumentation docs? Could you develop on why this needs to be a go-contrib instrumentation, and can't be owned by you, or GitLab? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, thanks for your will in contributing to OTel 👍
This proposes a new module and it lacks a code owner.
This PR won't be accepted (and possibly even reviewed) if no code owner is found.
Please check https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/CONTRIBUTING.md#code-owners for more information.
I created #6762 I think that we cannot expect contributors to find https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/instrumentation/README.md#new-instrumentation |
Thanks @dmathieu and @pellared for pointing those good points. I'm looking for both cases (gitlab pipelines otel observability and go test observability with tracing) and will be happy to contribute as much I can into it, but as currently not a part of a working group and do not participate SIG's - may not be a proper maintainer. Let's ask @dnsmichi, @sguyon opinion on this as Gitlab representatives. |
Fixes for Gitlab CI: #6759
This PR adds a resource detector for Gitlab CI which sets values according to to semantic conventions for cicd](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/attributes-registry/cicd.md) and vcs resources:
cicd.pipeline.name
cicd.pipeline.task.run.id
cicd.pipeline.task.name
cicd.pipeline.task.type
cicd.pipeline.run.id
cicd.pipeline.task.run.url.full
vcs.repository.ref.name
vcs.repository.ref.type
vcs.repository.change.id
vcs.repository.url.full