Skip to content

Commit

Permalink
Merge pull request #231 from bugsnag/PLAT-12425
Browse files Browse the repository at this point in the history
Change async event delivery to single goroutine with events channel queue
  • Loading branch information
DariaKunoichi authored Aug 5, 2024
2 parents 245bce0 + cf0947f commit c7a0901
Show file tree
Hide file tree
Showing 12 changed files with 169 additions and 36 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Changelog

## TBD

### Enhancements

* Limit resource usage while sending events asynchronously \
Added MainContext configuration option for providing context from main app
[#231](https://github.com/bugsnag/bugsnag-go/pull/231)

## 2.4.0 (2024-04-15)

### Enhancements
Expand Down
29 changes: 29 additions & 0 deletions LOCAL_TESTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@

## Unit tests
* Install old golang version (do not install just 1.11 - it's not compatible with running newer modules):

```
ASDF_GOLANG_OVERWRITE_ARCH=amd64 asdf install golang 1.11.13
```

* If you see error below use `CGO_ENABLED=0`.

```
# crypto/x509
malformed DWARF TagVariable entry
```

## Local testing with maze runner

* Maze runner tests require
* Specyfing `GO_VERSION` env variable to set a golang version for docker container.
* Ruby 2.7.
* Running docker.

* Commands to run tests

```
bundle install
bundle exec bugsnag-maze-runner
bundle exec bugsnag-maze-runner -c features/<chosen_feature>
```
4 changes: 3 additions & 1 deletion features/fixtures/app/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
ARG GO_VERSION
FROM golang:${GO_VERSION}-alpine

RUN apk update && apk upgrade && apk add git bash
RUN apk update && apk upgrade && apk add git bash build-base

ENV GOPATH /app

Expand All @@ -28,6 +28,8 @@ WORKDIR /app/src/test
# Skip on old versions of Go which pre-date modules
RUN if [[ $GO_VERSION != '1.11' && $GO_VERSION != '1.12' ]]; then \
go mod init && go mod tidy; \
echo "replace github.com/bugsnag/bugsnag-go/v2 => /app/src/github.com/bugsnag/bugsnag-go/v2" >> go.mod; \
go mod tidy; \
fi

RUN chmod +x run.sh
Expand Down
15 changes: 9 additions & 6 deletions features/fixtures/app/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ import (
bugsnag "github.com/bugsnag/bugsnag-go/v2"
)

func configureBasicBugsnag(testcase string) {
func configureBasicBugsnag(testcase string, ctx context.Context) {
config := bugsnag.Configuration{
APIKey: os.Getenv("API_KEY"),
AppVersion: os.Getenv("APP_VERSION"),
AppType: os.Getenv("APP_TYPE"),
Hostname: os.Getenv("HOSTNAME"),
APIKey: os.Getenv("API_KEY"),
AppVersion: os.Getenv("APP_VERSION"),
AppType: os.Getenv("APP_TYPE"),
Hostname: os.Getenv("HOSTNAME"),
MainContext: ctx,
}

if notifyReleaseStages := os.Getenv("NOTIFY_RELEASE_STAGES"); notifyReleaseStages != "" {
Expand Down Expand Up @@ -63,11 +64,13 @@ func configureBasicBugsnag(testcase string) {
}

func main() {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

test := flag.String("test", "handled", "what the app should send, either handled, unhandled, session, autonotify")
flag.Parse()

configureBasicBugsnag(*test)
configureBasicBugsnag(*test, ctx)
time.Sleep(100 * time.Millisecond) // Ensure tests are less flaky by ensuring the start-up session gets sent

switch *test {
Expand Down
4 changes: 3 additions & 1 deletion features/fixtures/autoconfigure/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
ARG GO_VERSION
FROM golang:${GO_VERSION}-alpine

RUN apk update && apk upgrade && apk add git bash
RUN apk update && apk upgrade && apk add git bash build-base

ENV GOPATH /app

Expand All @@ -28,6 +28,8 @@ WORKDIR /app/src/test
# Skip on old versions of Go which pre-date modules
RUN if [[ $GO_VERSION != '1.11' && $GO_VERSION != '1.12' ]]; then \
go mod init && go mod tidy; \
echo "replace github.com/bugsnag/bugsnag-go/v2 => /app/src/github.com/bugsnag/bugsnag-go/v2" >> go.mod; \
go mod tidy; \
fi

RUN chmod +x run.sh
Expand Down
4 changes: 3 additions & 1 deletion features/fixtures/net_http/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ FROM golang:${GO_VERSION}-alpine

RUN apk update && \
apk upgrade && \
apk add git
apk add git build-base

ENV GOPATH /app

Expand All @@ -30,4 +30,6 @@ WORKDIR /app/src/test
# Skip on old versions of Go which pre-date modules
RUN if [[ $GO_VERSION != '1.11' && $GO_VERSION != '1.12' ]]; then \
go mod init && go mod tidy; \
echo "replace github.com/bugsnag/bugsnag-go/v2 => /app/src/github.com/bugsnag/bugsnag-go/v2" >> go.mod; \
go mod tidy; \
fi
13 changes: 8 additions & 5 deletions features/fixtures/net_http/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import (
)

func main() {
configureBasicBugsnag()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
configureBasicBugsnag(ctx)

http.HandleFunc("/handled", handledError)
http.HandleFunc("/autonotify-then-recover", unhandledCrash)
Expand All @@ -40,16 +42,17 @@ func recoverWrap(h http.Handler) http.Handler {
})
}

func configureBasicBugsnag() {
func configureBasicBugsnag(ctx context.Context) {
config := bugsnag.Configuration{
APIKey: os.Getenv("API_KEY"),
Endpoints: bugsnag.Endpoints{
Notify: os.Getenv("BUGSNAG_ENDPOINT"),
Sessions: os.Getenv("BUGSNAG_ENDPOINT"),
},
AppVersion: os.Getenv("APP_VERSION"),
AppType: os.Getenv("APP_TYPE"),
Hostname: os.Getenv("HOSTNAME"),
AppVersion: os.Getenv("APP_VERSION"),
AppType: os.Getenv("APP_TYPE"),
Hostname: os.Getenv("HOSTNAME"),
MainContext: ctx,
}

if notifyReleaseStages := os.Getenv("NOTIFY_RELEASE_STAGES"); notifyReleaseStages != "" {
Expand Down
10 changes: 5 additions & 5 deletions features/handled.feature
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Scenario: Sending an event using a callback to modify report contents
And the event "severityReason.type" equals "userCallbackSetSeverity"
And the event "context" equals "nonfatal.go:14"
And the "file" of stack frame 0 equals "main.go"
And stack frame 0 contains a local function spanning 242 to 248
And stack frame 0 contains a local function spanning 245 to 253
And the "file" of stack frame 1 equals ">insertion<"
And the "lineNumber" of stack frame 1 equals 0

Expand All @@ -50,7 +50,7 @@ Scenario: Marking an error as unhandled in a callback
And the event "severityReason.type" equals "userCallbackSetSeverity"
And the event "severityReason.unhandledOverridden" is true
And the "file" of stack frame 0 equals "main.go"
And stack frame 0 contains a local function spanning 254 to 257
And stack frame 0 contains a local function spanning 257 to 262

Scenario: Unwrapping the causes of a handled error
When I run the go service "app" with the test case "nested-error"
Expand All @@ -59,12 +59,12 @@ Scenario: Unwrapping the causes of a handled error
And the event "unhandled" is false
And the event "severity" equals "warning"
And the event "exceptions.0.message" equals "terminate process"
And the "lineNumber" of stack frame 0 equals 292
And the "lineNumber" of stack frame 0 equals 295
And the "file" of stack frame 0 equals "main.go"
And the "method" of stack frame 0 equals "nestedHandledError"
And the event "exceptions.1.message" equals "login failed"
And the event "exceptions.1.stacktrace.0.file" equals "main.go"
And the event "exceptions.1.stacktrace.0.lineNumber" equals 312
And the event "exceptions.1.stacktrace.0.lineNumber" equals 315
And the event "exceptions.2.message" equals "invalid token"
And the event "exceptions.2.stacktrace.0.file" equals "main.go"
And the event "exceptions.2.stacktrace.0.lineNumber" equals 320
And the event "exceptions.2.stacktrace.0.lineNumber" equals 323
14 changes: 11 additions & 3 deletions v2/bugsnag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ import (
"github.com/bugsnag/bugsnag-go/v2/sessions"
)

// ATTENTION - tests in this file are changing global state variables
// like default config or default report publisher
// TAKE CARE to reset them to default after testcase!

// The line numbers of this method are used in tests.
// If you move this function you'll have to change tests
func crashyHandler(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -132,7 +136,7 @@ func TestNotify(t *testing.T) {
}

exception := getIndex(event, "exceptions", 0)
verifyExistsInStackTrace(t, exception, &StackFrame{File: "bugsnag_test.go", Method: "TestNotify", LineNumber: 98, InProject: true})
verifyExistsInStackTrace(t, exception, &StackFrame{File: "bugsnag_test.go", Method: "TestNotify", LineNumber: 102, InProject: true})
}

type testPublisher struct {
Expand All @@ -144,6 +148,9 @@ func (tp *testPublisher) publishReport(p *payload) error {
return nil
}

func (tp *testPublisher) setMainProgramContext(context.Context) {
}

func TestNotifySyncThenAsync(t *testing.T) {
ts, _ := setup()
defer ts.Close()
Expand All @@ -152,7 +159,7 @@ func TestNotifySyncThenAsync(t *testing.T) {

pub := new(testPublisher)
publisher = pub
defer func() { publisher = new(defaultReportPublisher) }()
defer func() { publisher = newPublisher() }()

Notify(fmt.Errorf("oopsie"))
if pub.sync {
Expand All @@ -175,6 +182,7 @@ func TestHandlerFunc(t *testing.T) {
defer eventserver.Close()
Configure(generateSampleConfig(eventserver.URL))

// NOTE - this testcase will print a panic in verbose mode
t.Run("unhandled", func(st *testing.T) {
sessionTracker = nil
startSessionTracking()
Expand Down Expand Up @@ -315,7 +323,7 @@ func TestHandler(t *testing.T) {
}

exception := getIndex(event, "exceptions", 0)
verifyExistsInStackTrace(t, exception, &StackFrame{File: "bugsnag_test.go", Method: "crashyHandler", InProject: true, LineNumber: 24})
verifyExistsInStackTrace(t, exception, &StackFrame{File: "bugsnag_test.go", Method: "crashyHandler", InProject: true, LineNumber: 28})
}

func TestAutoNotify(t *testing.T) {
Expand Down
11 changes: 11 additions & 0 deletions v2/configuration.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package bugsnag

import (
"context"
"log"
"net/http"
"os"
Expand Down Expand Up @@ -101,6 +102,12 @@ type Configuration struct {
// Whether bugsnag should notify synchronously. This defaults to false which
// causes bugsnag-go to spawn a new goroutine for each notification.
Synchronous bool

// Context created in the main program
// Used in event delivery - after this context is marked Done
// the event sending goroutine will switch to a graceful shutdown
// and will try to send any remaining events.
MainContext context.Context
// Whether the notifier should send all sessions recorded so far to Bugsnag
// when repanicking to ensure that no session information is lost in a
// fatal crash.
Expand Down Expand Up @@ -160,6 +167,10 @@ func (config *Configuration) update(other *Configuration) *Configuration {
if other.Synchronous {
config.Synchronous = true
}
if other.MainContext != nil {
config.MainContext = other.MainContext
publisher.setMainProgramContext(other.MainContext)
}

if other.AutoCaptureSessions != nil {
config.AutoCaptureSessions = other.AutoCaptureSessions
Expand Down
11 changes: 6 additions & 5 deletions v2/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"github.com/bugsnag/bugsnag-go/v2/errors"
)

var publisher reportPublisher = new(defaultReportPublisher)
var publisher reportPublisher = newPublisher()

// Notifier sends errors to Bugsnag.
type Notifier struct {
Expand Down Expand Up @@ -84,10 +84,11 @@ func (notifier *Notifier) NotifySync(err error, sync bool, rawData ...interface{
// AutoNotify notifies Bugsnag of any panics, then repanics.
// It sends along any rawData that gets passed in.
// Usage:
// go func() {
// defer AutoNotify()
// // (possibly crashy code)
// }()
//
// go func() {
// defer AutoNotify()
// // (possibly crashy code)
// }()
func (notifier *Notifier) AutoNotify(rawData ...interface{}) {
if err := recover(); err != nil {
severity := notifier.getDefaultSeverity(rawData, SeverityError)
Expand Down
Loading

0 comments on commit c7a0901

Please sign in to comment.