Skip to content
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

Set GenAi subtype for spans that have the gen_ai.system attribute #127

Merged
merged 5 commits into from
Dec 16, 2024

Conversation

xrmx
Copy link
Member

@xrmx xrmx commented Dec 3, 2024

Would like to avoid spans with unknown as type in kibana for genai libraries traced calls.

Schermata del 2024-12-03 14-45-57

This has only been unit tested here, no clue on how to run this :)

Closes #111

@xrmx xrmx force-pushed the external-genai-sybtype branch 2 times, most recently from 67ba8e1 to 1b89ccb Compare December 3, 2024 13:37
@xrmx xrmx changed the title Set GenAi subtype for spans Set GenAi subtype for spans that have the gen_ai.system attribute Dec 3, 2024
@xrmx xrmx force-pushed the external-genai-sybtype branch from 1b89ccb to 5b54577 Compare December 3, 2024 13:41
@xrmx xrmx marked this pull request as ready for review December 3, 2024 13:41
@xrmx xrmx requested a review from a team as a code owner December 3, 2024 13:41
Copy link
Contributor

@gregkalapos gregkalapos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense.

Only reason this wasn't here is that this logic pre-dates popularity of genai.

Other side note: this code will be used within the OTel collector, but won't be used in APM Server/MIS, right?

I'd say that's fine, just flagging this, because until now, EDOT collector and APM Server/MIS behaved very similarly and here they'll diverge, which may be strange for users.

Comment on lines +36 to +37
semconv25 "go.opentelemetry.io/collector/semconv/v1.25.0"
semconv27 "go.opentelemetry.io/collector/semconv/v1.27.0"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and so it begins... 😛

@xrmx
Copy link
Member Author

xrmx commented Dec 13, 2024

With the help of @gregkalapos I've setup things to test this but I'm not able to make this work locally and I get an unknown span.

These are the local changes I have in elastic-collector-components in order to test this:

diff --git a/distributions/elastic-components/manifest.yaml b/distributions/elastic-components/manifest.yaml
index b29bf5e..062405c 100644
--- a/distributions/elastic-components/manifest.yaml
+++ b/distributions/elastic-components/manifest.yaml
@@ -10,6 +10,7 @@ extensions:
   - gomod: github.com/open-telemetry/opentelemetry-collector-contrib/extension/pprofextension v0.114.0
   - gomod: github.com/open-telemetry/opentelemetry-collector-contrib/extension/storage/filestorage v0.114.0
   - gomod: go.opentelemetry.io/collector/extension/memorylimiterextension v0.114.0
+  - gomod: github.com/open-telemetry/opentelemetry-collector-contrib/extension/basicauthextension v0.114.0
 
 connectors:
   - gomod: github.com/elastic/opentelemetry-collector-components/connector/signaltometricsconnector v0.0.0
diff --git a/processor/elastictraceprocessor/go.mod b/processor/elastictraceprocessor/go.mod
index fc8bd49..29cd501 100644
--- a/processor/elastictraceprocessor/go.mod
+++ b/processor/elastictraceprocessor/go.mod
@@ -19,6 +19,8 @@ require (
        go.uber.org/zap v1.27.0
 )
 
+replace github.com/elastic/opentelemetry-lib => /home/rm/src/opentelemetry-lib
+
 require (
        github.com/cespare/xxhash/v2 v2.3.0 // indirect
        github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect

Do you see something I am missing?

@lahsivjar
Copy link
Contributor

Do you see something I am missing?

If you are building ocb image then you will need to add replace directive in the ocb config (something like https://github.com/elastic/otel-apm-e2e-poc/blob/f45432edc2613f8997caed7c917ae71f5dec8d4d/ocb.yaml#L32) and then build the custom otel collector to do the testing.

@codefromthecrypt
Copy link

Curious about the timeline required to get this into the EDOT collector by default once merged? Is it feasible in Jan? (thinking about public demos and/or blogs)

@xrmx
Copy link
Member Author

xrmx commented Dec 16, 2024

Do you see something I am missing?

If you are building ocb image then you will need to add replace directive in the ocb config (something like https://github.com/elastic/otel-apm-e2e-poc/blob/f45432edc2613f8997caed7c917ae71f5dec8d4d/ocb.yaml#L32) and then build the custom otel collector to do the testing.

Thanks but I get this error. Tried both building v0.114.0 in elastic-collector-components or v0.111.0 in otel-apm-e2e-poc.

Error: failed to update go.mod: go subcommand failed with args '[mod tidy -compat=1.22]': exit status 1, error message: go: finding module for package go.opentelemetry.io/collector/pdata/internal/data/protogen/collector/profiles/v1experimental
go: finding module for package go.opentelemetry.io/collector/pdata/internal/data/protogen/profiles/v1experimental
go: go.opentelemetry.io/collector/cmd/builder imports
	go.opentelemetry.io/collector/receiver/otlpreceiver imports
	go.opentelemetry.io/collector/pdata/pprofile/pprofileotlp imports
	go.opentelemetry.io/collector/pdata/internal/data/protogen/collector/profiles/v1experimental: module go.opentelemetry.io/collector/pdata@latest found (v1.21.0), but does not contain package go.opentelemetry.io/collector/pdata/internal/data/protogen/collector/profiles/v1experimental
go: go.opentelemetry.io/collector/cmd/builder imports
	go.opentelemetry.io/collector/connector imports
	go.opentelemetry.io/collector/internal/fanoutconsumer imports
	go.opentelemetry.io/collector/pdata/pprofile imports
	go.opentelemetry.io/collector/pdata/internal/data/protogen/profiles/v1experimental: module go.opentelemetry.io/collector/pdata@latest found (v1.21.0), but does not contain package go.opentelemetry.io/collector/pdata/internal/data/protogen/profiles/v1experimental

@lahsivjar
Copy link
Contributor

or v0.111.0 in otel-apm-e2e-poc

Hmm, might be something to do with version up, looks like the otel-e2e-poc has not been updated and otel lib is already using v0.115.0. Let me see if I can do a quick fix there.

@xrmx
Copy link
Member Author

xrmx commented Dec 16, 2024

🎉

Schermata del 2024-12-16 14-42-32

With https://github.com/elastic/otel-apm-e2e-poc/commit/21e7ef1849b6f45e8a0dea788746de6caee0e5ca and by remembering to not start the apm-server works fine. Thanks @lahsivjar and @gregkalapos for the help!

@xrmx xrmx merged commit cfe2ddc into main Dec 16, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unknown type for gen_ai spans
5 participants