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.

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.

It looks like somehow all the files suddenly got double-spaced and the linter is not happy about it. I don't think the previous version had the double-spacing issue.

@merchantmoh-debug
Copy link
Contributor Author

@gmlewis - Yup. the files got converted to CRLF within antigravity introducing double lines. Fixed the issue so it stops happening. I'll recommit in a moment.

@merchantmoh-debug
Copy link
Contributor Author

Tactical Tip (The "Never Again" Lock): Before you push, run this to strip the ghosts: gofmt -w -l . (Standard Go formatter usually fixes line endings too). OR find . -type f -name "*.go" -exec dos2unix {} +

---- Hopefully that "tactical tip" my AI gave me worked!

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.

Unfortunately, github/github.go is pretty mangled now. You probably want to redo the changes to this file.

@merchantmoh-debug
Copy link
Contributor Author

@gmlewis - Yeah. I'm trying to figure out a way out of this cycle - I hate getting stuck at the finish line.

I'll hang up my hat and beg your help after a couple more tries 😂

@merchantmoh-debug merchantmoh-debug force-pushed the feat/opentelemetry-support branch 4 times, most recently from 21a1109 to 85f80ce Compare January 29, 2026 02:43
@merchantmoh-debug
Copy link
Contributor Author

@gmlewis - Hold off the retest. I figured out the problem. I found the write tests. I'll hit review when I'm ready.

@merchantmoh-debug
Copy link
Contributor Author

@gmlewis - Alright. If this works it works. If not - feel free to suggest the fixes and I'll commit. I'm heading to bed now but I should be awake for at least 30 more minutes.

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.

Unfortunately, 28 files have now been edited when we were down to the correct 8 two commits ago. If you revert the changes to 20 of the files, we can take another look.

@merchantmoh-debug merchantmoh-debug force-pushed the feat/opentelemetry-support branch from 7b262f6 to 2ac81f8 Compare January 29, 2026 15:15
@merchantmoh-debug
Copy link
Contributor Author

@gmlewis - Should be good to go now. Feel free to do the tests and suggest any fixes if any come up.

My stubborn desire to preserve automation has led me to much success in extreme productivity - but at the price of idiotic errors that make me want to hide my face in embarrassment. The price of progress?

Thanks for the patience @gmlewis - Let's get this one over with so I can finish the Iterator & go from there.

:)

merchantmoh-debug and others added 6 commits January 29, 2026 15:25
Co-authored-by: Oleksandr Redko <[email protected]>
Co-authored-by: Oleksandr Redko <[email protected]>
Co-authored-by: Oleksandr Redko <[email protected]>
Co-authored-by: Oleksandr Redko <[email protected]>
Co-authored-by: Oleksandr Redko <[email protected]>
Co-authored-by: Oleksandr Redko <[email protected]>
@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.

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