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

Golang Getting started - based on roll dice service and Automatic Ins… #3244

Closed
wants to merge 1 commit into from

Conversation

pegasas
Copy link
Contributor

@pegasas pegasas commented Sep 9, 2023

try to add an roll dice example with golang, solving #2623

@pegasas pegasas marked this pull request as ready for review September 9, 2023 17:53
@pegasas pegasas requested review from a team September 9, 2023 17:53
Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

This changes the substance of this documentation from manual instrumentation to automatic instrumentation. Automatic instrumentation deserves to live besides this not in-place of it.

@svrnm
Copy link
Member

svrnm commented Sep 11, 2023

thanks for raising this @pegasas. This still looks work-in-progress for me, so please put this into draft mode for now.

This changes the substance of this documentation from manual instrumentation to automatic instrumentation. Automatic instrumentation deserves to live besides this not in-place of it.

Right now I don't see how the documentation has been changed from manual to automatic instrumentation, as it seems incomplete. To comment on that nevertheless, as we have a discussion on the same ongoing over here: _ the assumption is that a Getting Started should give you something tangible as quick as possible in your preferred language_, so if the Golang SIG at some point comes to the conclustion that automatic instrumentation might be a better route to that, we can adopt it in the Getting Started, and have a similar structure as we have it for Java & JavaScript these days (Getting Started using mostly automation, have a "manual" and "automatic" documentation).

@pegasas pegasas marked this pull request as draft September 11, 2023 08:18
@pegasas
Copy link
Contributor Author

pegasas commented Sep 11, 2023

@MrAlias @svrnm

Sure.
I will push out again when I resolve automatic instrumentation.

The `Fibonacci()` function intentionally produces invalid results for
sufficiently large values of `n`. This is addressed in
[Error handling](#error-handling).
This page will show you how to get started with OpenTelemetry in Golang.
Copy link
Member

Choose a reason for hiding this comment

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

The language is called "Go". Not "Golang" 😉

Comment on lines +21 to +24
The following example uses a basic
[Gin Web Framework](https://gin-gonic.com/docs/introduction/). If you are
not using Gin, that's OK — you can use OpenTelemetry Golang with other web
frameworks as well, such as Goframe and Beego.
Copy link
Member

@pellared pellared Sep 11, 2023

Choose a reason for hiding this comment

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

Can we simply use net/http? You can mention that it works for Gin, Gorilla/Mux, Echo etc.

I would say that using HTTP frameworks is often NOT seen as idiomatic. Also the example is so simple that it does not need any "framework" features. At last, we thrive to have the best support for http/net. We even consider dropping support for other libraries.

```go
go mod init
go get github.com/gin-gonic/gin
go mod vendor
Copy link
Member

Choose a reason for hiding this comment

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

Vendoring is not needed.

@pellared
Copy link
Member

I will push out again when I resolve automatic instrumentation.

What do you mean? Right now, getting started should work with manual instrumentation.

@cartermp
Copy link
Contributor

So I think we should change this eventually to use autoinstrumentation first, but the main issue is that the Go autoinstrumentation agent is still quite early. I don't know if we should endorse it as the right way to get started today.

@pegasas
Copy link
Contributor Author

pegasas commented Sep 12, 2023

So I think we should change this eventually to use autoinstrumentation first, but the main issue is that the Go autoinstrumentation agent is still quite early. I don't know if we should endorse it as the right way to get started today.

https://github.com/open-telemetry/opentelemetry-go-instrumentation
Indeed,
It seems this project is progressing.
Is this the only project which adapted eBPF?

@cartermp
Copy link
Contributor

Yes, for now it is. There may be more in the future though, since the technique could also apply for Rust and C++. But I don't know if there's any motivation on those SIGs to start an autoinstrumentation project.

@pellared
Copy link
Member

Is this the only project which adapted eBPF?

There is also https://github.com/open-telemetry/opentelemetry-ebpf

@svrnm
Copy link
Member

svrnm commented Oct 5, 2023

@pegasas there is now an updated getting started for go using manual instrumentation. Do you still have interest in writing something about automatic instrumentation or can we close this PR?

@pegasas
Copy link
Contributor Author

pegasas commented Oct 17, 2023

sure. @svrnm .

I close this PR.
I am interested in other items in automatic instrumentation.
Do you mean #3345?

@pegasas pegasas closed this Oct 17, 2023
@svrnm
Copy link
Member

svrnm commented Oct 17, 2023

@pegasas my question was around having some section added to the go lang documentation around automatic instrumentation. #3345 is not about Go and not about Automatic instrumentation, can you provide more details?

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.

5 participants