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

Support gRPC status for versions >=1.40 #1235

Merged
merged 4 commits into from
Nov 1, 2024

Conversation

damemi
Copy link
Contributor

@damemi damemi commented Oct 31, 2024

The Status fields for gRPC have changed several times. Since we only recently added support for parsing the gRPC status into auto-instrumentation, I think it is reasonable to add a gate to only support the Status for recent versions (that have the same field format), rather than adding various checks for every version of gRPC.

Working on testing this manually

Fixes #1225

@damemi damemi requested a review from a team as a code owner October 31, 2024 17:12
@MrAlias
Copy link
Contributor

MrAlias commented Oct 31, 2024

Can you add a changelog mentioning the fix and what support restrictions this introduces?

@damemi
Copy link
Contributor Author

damemi commented Nov 1, 2024

I was able to manually test this with a grpc example and realized the error was coming from earlier than my first commits, so I switched to using StructFieldConstMinVersion (thanks @RonFed! forgot we had that)

Writing down my repro steps:

Tested by cloning https://github.com/grpc/grpc-go/tree/v1.39.x and building an image with this Dockerfile:

FROM golang:latest

RUN mkdir -p /app

WORKDIR /app

COPY . .

ENV GOPATH /app

RUN cd examples/helloworld/greeter_client && go install .
RUN cd examples/helloworld/greeter_server && go install .

Then loaded that image into a kind cluster and created this pod (along with a jaeger service):

apiVersion: v1
kind: Pod
metadata:
  name: grpc
spec:
  shareProcessNamespace: true
  containers:
  - name: grpc-server
    image: grpc-test
    imagePullPolicy: Never
    command: ['/app/bin/greeter_server']
    ports:
    - containerPort: 3000
  - name: auto-instrumentation
    image: otel-go-instrumentation
    imagePullPolicy: Never
    securityContext:
      runAsUser: 0
      privileged: true
    env:
      - name: OTEL_GO_AUTO_TARGET_EXE
        value: /app/bin/greeter_server
      - name: OTEL_EXPORTER_OTLP_ENDPOINT
        value: "http://jaeger:4318"
      - name: OTEL_SERVICE_NAME
        value: "grpc"

Without 6fa573c, I got the same error from the auto-instrumentation pod as reported in #1225 (comment). After adding that commit, it seems to work:

{"time":"2024-11-01T02:17:29.996515721Z","level":"INFO","source":{"function":"main.main","file":"/app/cli/main.go","line":103},"msg":"building OpenTelemetry Go instrumentation ...","globalImpl":false,"version":{"Release":"v0.16.0-alpha","Revision":"d2dde82683c392f7c8ce97ba70b15bbf5e96d48f-dirty","Go":{"Version":"go1.23.2","OS":"linux","Arch":"arm64"}}}
{"time":"2024-11-01T02:17:31.99971693Z","level":"INFO","source":{"function":"go.opentelemetry.io/auto/internal/pkg/process.(*Analyzer).DiscoverProcessID","file":"/app/internal/pkg/process/discover.go","line":64},"msg":"found process","pid":7}
{"time":"2024-11-01T02:17:32.009699013Z","level":"INFO","source":{"function":"go.opentelemetry.io/auto/internal/pkg/process.remoteAllocate.func1","file":"/app/internal/pkg/process/allocate.go","line":70},"msg":"Detaching from process","pid":7}
{"time":"2024-11-01T02:17:32.009782138Z","level":"INFO","source":{"function":"go.opentelemetry.io/auto.NewInstrumentation","file":"/app/instrumentation.go","line":113},"msg":"target process analysis completed","pid":7,"go_version":"1.23.2","dependencies":{"github.com/golang/protobuf":"1.4.3","golang.org/x/net":"0.0.0-20200822124328-c89045814202","golang.org/x/sys":"0.0.0-20200323222414-85ca7c5b95cd","golang.org/x/text":"0.3.0","google.golang.org/genproto":"0.0.0-20200806141610-86f49bd18e98","google.golang.org/grpc":"1.36.0","google.golang.org/protobuf":"1.25.0","std":"1.23.2"},"total_functions_found":7}
{"time":"2024-11-01T02:17:32.00997893Z","level":"INFO","source":{"function":"main.main","file":"/app/cli/main.go","line":134},"msg":"starting instrumentation..."}
{"time":"2024-11-01T02:17:32.01161443Z","level":"INFO","source":{"function":"go.opentelemetry.io/auto/internal/pkg/instrumentation.(*Manager).load","file":"/app/internal/pkg/instrumentation/manager.go","line":329},"msg":"loading probe","name":{"SpanKind":3,"InstrumentedPkg":"google.golang.org/grpc"}}
{"time":"2024-11-01T02:17:32.085941972Z","level":"INFO","source":{"function":"go.opentelemetry.io/auto/internal/pkg/instrumentation.(*Manager).load","file":"/app/internal/pkg/instrumentation/manager.go","line":329},"msg":"loading probe","name":{"SpanKind":2,"InstrumentedPkg":"google.golang.org/grpc"}}
{"time":"2024-11-01T02:17:32.114058097Z","level":"INFO","source":{"function":"main.main.func3","file":"/app/cli/main.go","line":130},"msg":"instrumentation loaded successfully"}
{"time":"2024-11-01T02:17:32.11409118Z","level":"INFO","source":{"function":"go.opentelemetry.io/auto/internal/pkg/instrumentation.(*Manager).ConfigLoop","file":"/app/internal/pkg/instrumentation/manager.go","line":238},"msg":"Configuration provider closed, configuration updates will no longer be received"}

@damemi damemi merged commit 1151ce9 into open-telemetry:main Nov 1, 2024
26 checks passed
@damemi damemi deleted the issue-1225 branch November 1, 2024 19:07
@MrAlias MrAlias added this to the v0.17.0-alpha milestone Nov 4, 2024
@MrAlias MrAlias mentioned this pull request Nov 5, 2024
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.

It needs to support retrieving status codes in gRPC client versions prior to v1.40.
3 participants