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

examples: Add OpenTelemetry example #7296

Merged
merged 1 commit into from
Jul 9, 2024
Merged

Conversation

zasweq
Copy link
Contributor

@zasweq zasweq commented Jun 3, 2024

This PR adds an OpenTelemetry instrumentation example.

The example is not tested (I have a commit that does have the test, but deleted it, so the diff can be seen). This is due to our examples test expecting the client to exit, which in this case the client cannot exit for the sake of the example, as it is exporting Prometheus metrics on a port. Let me know if you would prefer changing our examples test. Note that a CSM Observability example with exact same behavior expect configure OpenTelemetry through CSM Package will be coming right after this.

RELEASE NOTES: N/A

@zasweq zasweq added the Type: Documentation Documentation or examples label Jun 3, 2024
@zasweq zasweq added this to the 1.65 Release milestone Jun 3, 2024
@zasweq zasweq requested a review from easwars June 4, 2024 15:44
@easwars
Copy link
Contributor

easwars commented Jun 7, 2024

Given that OTel is a separate submodule, why not have the example inside that module? We take that approach for advancedtls I believe.

@zasweq
Copy link
Contributor Author

zasweq commented Jun 7, 2024

Hmmmm sure.

examples/features/opentelemetry/client/main.go Outdated Show resolved Hide resolved
examples/features/opentelemetry/client/main.go Outdated Show resolved Hide resolved
examples/features/opentelemetry/server/main.go Outdated Show resolved Hide resolved
examples/features/opentelemetry/server/main.go Outdated Show resolved Hide resolved
examples/features/opentelemetry/server/main.go Outdated Show resolved Hide resolved
examples/features/opentelemetry/server/main.go Outdated Show resolved Hide resolved
examples/features/opentelemetry/server/main.go Outdated Show resolved Hide resolved
examples/features/opentelemetry/server/main.go Outdated Show resolved Hide resolved
@easwars easwars assigned zasweq and unassigned easwars Jun 7, 2024
@zasweq
Copy link
Contributor Author

zasweq commented Jun 10, 2024

There is a CSM Example in flight that we plugged into UG that is similar to this; I think it would be best to wait for our full back and forth on that and then incorporate all changes there into this. I already did some changes such as flags to align with UG and also relaxing the RPC failure on client side to a Info vs. Error to keep the binary running even if RPC's fail. That way, erroring RPC's can also be seen through Prometheus (say if the Server Restarts on Kubernetes for CSM Observability example, or locally for this example), and doesn't require to restart binary (on Kubernetes for CSM Observability example and locally for this example.)

@zasweq zasweq assigned easwars and unassigned zasweq Jun 27, 2024
@easwars easwars assigned zasweq and unassigned easwars Jul 2, 2024
Copy link

codecov bot commented Jul 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.28%. Comparing base (bb49a88) to head (7ef4259).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7296      +/-   ##
==========================================
- Coverage   81.33%   81.28%   -0.05%     
==========================================
  Files         348      348              
  Lines       26744    26744              
==========================================
- Hits        21752    21739      -13     
- Misses       3795     3804       +9     
- Partials     1197     1201       +4     

see 11 files with indirect coverage changes

@zasweq zasweq merged commit daab563 into grpc:master Jul 9, 2024
12 of 13 checks passed
printchard pushed a commit to printchard/grpc-go that referenced this pull request Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Documentation Documentation or examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants