Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add dice example #4539

Merged
merged 21 commits into from
Sep 26, 2023
Merged

Add dice example #4539

merged 21 commits into from
Sep 26, 2023

Conversation

pellared
Copy link
Member

@pellared pellared commented Sep 21, 2023

Towards #4531

Mostly based on https://opentelemetry.io/docs/instrumentation/python/getting-started/ (but also looked at other languages)

Remarks

I did my best to make the example reasonably close to “real production code” (however, without tests).

I started with

package main

import (
	"io"
	"log"
	"math/rand"
	"net/http"
	"strconv"
)

func main() {
	http.HandleFunc("/rolldice", func(w http.ResponseWriter, r *http.Request) {
		roll := 1 + rand.Intn(6)
		resp := strconv.Itoa(roll) + "\n"
		if _, err := io.WriteString(w, resp); err != nil {
			log.Printf("Write failed: %v\n", err)
		}
	})

	log.Fatal(http.ListenAndServe(":8080", nil))
}

But I needed graceful shutdown (via SIGINT) to properly handle cleanup. Second SIGINT should kill the application.
I also decided to restructure the code and e.g. avoid using globals.
Moreover, I set server's WriteTimeout and ReadTimeout as it is a good practice.

@pellared
Copy link
Member Author

@svrnm @cartermp PTAL

@svrnm
Copy link
Member

svrnm commented Sep 21, 2023

nice work @pellared

@pellared pellared requested a review from dmathieu September 21, 2023 15:13
pellared and others added 2 commits September 21, 2023 21:50
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@pellared pellared requested a review from MrAlias September 21, 2023 19:58
pellared and others added 2 commits September 22, 2023 19:32
Co-authored-by: Phillip Carter <pcarter@fastmail.com>
@pellared
Copy link
Member Author

93a69bc is addressing comments from open-telemetry/opentelemetry.io#3310 (review)

@pellared
Copy link
Member Author

pellared commented Sep 26, 2023

@dashpole Can you please take a quick look as I think you already reviewed this code? 😉

@MrAlias MrAlias merged commit 0425c09 into open-telemetry:main Sep 26, 2023
@pellared pellared deleted the dice branch September 26, 2023 17:51
@MrAlias MrAlias added this to the v1.19.0/v0.42.0 milestone Sep 28, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants