Skip to content

Conversation

@merchantmoh-debug
Copy link
Contributor

Adds a new optional module github.com/google/go-github/v81/otel which provides an http.RoundTripper instrumented with OpenTelemetry.

Features

  • Automatic Tracing: Every API call gets a semantic span (e.g., github/GET).
  • Rate Limit Observability: Parses GitHub headers (X-RateLimit-*) and records them as Span Attributes (github.rate_limit.remaining, github.rate_limit.reset).
  • Support Tracing: Records X-Github-Request-Id for easy debugging with GitHub Support.

Verification

Validated with otel/example app against the live API.
Dependencies are isolated in a nested module to avoid bloating core go-github.

@merchantmoh-debug
Copy link
Contributor Author

@gmlewis - Surprise!

Haha. Please review.

@gmlewis
Copy link
Collaborator

gmlewis commented Jan 24, 2026

@merchantmoh-debug - I'm starting to think I'm talking to a bot.
This is a duplicate of #3916 and the description and subject are wrong.

@merchantmoh-debug
Copy link
Contributor Author

@gmlewis - Yeah. My bad. It's on my local disk set up. I use antigravity heavily. It likely made an error.

I'll fix it.

@merchantmoh-debug merchantmoh-debug force-pushed the feat/opentelemetry-support branch from 1d9b307 to 4536640 Compare January 24, 2026 03:00
@merchantmoh-debug
Copy link
Contributor Author

@merchantmoh-debug - I'm starting to think I'm talking to a bot. This is a duplicate of #3916 and the description and subject are wrong.

A bot that codes 3x performance boosts to stringify?

Can I get access to that bot please?

@gmlewis gmlewis added the DO NOT MERGE Do not merge this PR. label Jan 26, 2026
@gmlewis
Copy link
Collaborator

gmlewis commented Jan 28, 2026

@merchantmoh-debug - are you planning on updating this PR as requested, or shall we close it since the PR title and description have nothing to do with the changes that have been made?

@merchantmoh-debug merchantmoh-debug force-pushed the feat/opentelemetry-support branch from 42cbea7 to 0b0ca7c Compare January 28, 2026 03:33
@merchantmoh-debug
Copy link
Contributor Author

@gmlewis - Fixed.

@merchantmoh-debug merchantmoh-debug force-pushed the feat/opentelemetry-support branch from 0b0ca7c to 19330d6 Compare January 28, 2026 03:52
@merchantmoh-debug
Copy link
Contributor Author

@gmlewis - Branch scrubbed and clean. This completes the modernization trilogy (Perf #3914, Iterators #3916, OTEL #3938).

Let's go 3 for 3.

Copy link
Contributor

@alexandear alexandear left a comment

Choose a reason for hiding this comment

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

Please fix lint issues: ./script/lint.sh.

@gmlewis gmlewis added NeedsReview PR is awaiting a review before merging. and removed DO NOT MERGE Do not merge this PR. labels Jan 28, 2026
@merchantmoh-debug merchantmoh-debug force-pushed the feat/opentelemetry-support branch from 19330d6 to c786a38 Compare January 28, 2026 16:49
@merchantmoh-debug
Copy link
Contributor Author

Thanks for the review @alexandear @gmlewis I've addressed all your feedback in the latest push:

Directory Structure: Moved otel/example to example/otel to align with repo conventions.

Standards: Added missing Copyright headers (2026) and updated the instrumentationName to v82.

Code Design: Exported HeaderRateReset in github/github.go

and updated transport.go to use this constant instead of the hardcoded string.

Maintenance: Added a sentinel // NOTE comment above instrumentationName to flag it for manual updates during release cycles.

Linting: Ran go mod tidy in both the root and example directories to resolve the go.sum validation errors.

Requesting review @gmlewis @alexandear

Copy link
Collaborator

@gmlewis gmlewis 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 we can completely remove otel/example/* since the example now lives in example/otel.
Please fix the linting and test errors (see CONTRIBUTING.md) and push the changes before we can fully review this PR.

@merchantmoh-debug merchantmoh-debug force-pushed the feat/opentelemetry-support branch from c786a38 to 9ea671d Compare January 28, 2026 18:18
@gmlewis
Copy link
Collaborator

gmlewis commented Jan 28, 2026

@merchantmoh-debug - you just force-pushed over 1000 files.

over-1000-files

@merchantmoh-debug
Copy link
Contributor Author

@gmlewis - Lmao. Antigravity is finicky. My bad. It's still a google labs product.... hahaha - I'm laughing my head off right now. "Over 1000" omg what did it push? my entire library? lmao.

I'll reverse it --- If the lint doesn't fix I might need your help figuring it out.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

@merchantmoh-debug - please remove the "go" folder.

I really wish you would review your PRs using the GitHub user interface before requesting code reviews as this is truly a waste of my time and patience.

@alexandear
Copy link
Contributor

@gmlewis - Lmao. Antigravity is finicky. My bad. It's still a google labs product.... hahaha.

Hey @merchantmoh-debug, could you manually edit and push changes by using git? (Here's a handly cheat sheet if you need it.) No pressure to use coding agents - totally up to you!

I'll reverse it --- If the lint doesn't fix I might need your help figuring it out.

No worries! You can fix lint issues easily by running ./lint/script.sh locally, checking the output, and applying any suggested changes. You can also refer to the Golangci-lint docs for more details.

@merchantmoh-debug
Copy link
Contributor Author

@gmlewis - Well seeing as I'm both using my time & patience to give google free upgrades to its library - - We both lose time when things to go right now don't we?

Yeah - once more; I'm still learning the ropes. Ever taught at a school? You don't get far with impatience & frustration. You have to be patient. Don't be fooled by the competence of my coding and assume I know what I'm doing fully. I don't.

I'm a unicorn - you'll have to delete any patterns you have on what's "normal" when dealing with me. I'm High-IQ Autistic, Non-Automatic TOMS < (I bet you can tell) > ADHD > OCD > OCPD.

I see the world in systems and logic. It makes me amazingly good at learning extremely complex things extremely quickly. But it often leads me to learn something like "How to be a badass coder" before I ever learn "How to navigate Githubs user-interface"

And I like to trial & error my way through problems - try, review, try, review - I WOULD do this automatically but the tests are set to only happen when you review. Forcing a human-in-the-loop situation where you get frustrated when I make an oppsie which will NOT stop happening right away.

It's the trade off of the neurodivergent my friend - super-smart but super-hard to deal with.

And me? I'm actually pretty good to deal with in comparison. 🤷‍♂️

@merchantmoh-debug
Copy link
Contributor Author

@gmlewis - Lmao. Antigravity is finicky. My bad. It's still a google labs product.... hahaha.

Hey @merchantmoh-debug, could you manually edit and push changes by using git? (Here's a handly cheat sheet if you need it.) No pressure to use coding agents - totally up to you!

I'll reverse it --- If the lint doesn't fix I might need your help figuring it out.

No worries! You can fix lint issues easily by running ./lint/script.sh locally, checking the output, and applying any suggested changes. You can also refer to the Golangci-lint docs for more details.

@alexandear - Thanks! I appreciate the guidance!

Yeah of course! No problem! I'll manually edit them! You're awesome.

@merchantmoh-debug merchantmoh-debug force-pushed the feat/opentelemetry-support branch from 9ea671d to 03b05e6 Compare January 28, 2026 19:01
@merchantmoh-debug
Copy link
Contributor Author

@alexandear - Thanks for the cheat sheet & the other link.

Huge help. Saves a lot of your guys time now that I can do the testing myself.

I'll update soon.

merchantmoh-debug and others added 6 commits January 29, 2026 15:25
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
@merchantmoh-debug
Copy link
Contributor Author

merchantmoh-debug commented Jan 29, 2026

@alexandear - Thanks for the chores - that's quite a smart addition to make contributions easier. Thanks as well for the suggestions.

All suggestions committed as requested.

@merchantmoh-debug
Copy link
Contributor Author

@gmlewis - Give it a go bro.

@merchantmoh-debug
Copy link
Contributor Author

@gmlewis @alexandear - I'm testing it extensively -- yet for some reason it keeps failing the github workflow even though it passes the local one.

Weird.

To decompress I made this song for us - https://www.youtube.com/watch?v=AL175r0MQ4E

Any ideas where we're going wrong?

@gmlewis
Copy link
Collaborator

gmlewis commented Jan 31, 2026

Any ideas where we're going wrong?

Yes, the copyright message wasn't exactly identical to other files and go fmt was not run on one of the files.
I'm trying to correct those issues now. Please stand by.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

OK, great... we are almost there.
Now we need a new file called "otel/transport_test.go" that will provide excellent code coverage over its corresponding "otel/transport.go" file and then we should hopefully be ready for a final review.

You will need to "git pull" the changes I made to your current branch to get the copyright and formatting changes locally so that you can continue.

// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// This example demonstrates ...
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfinished sentence. What this package demonstrates? 🙃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NeedsReview PR is awaiting a review before merging.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants