honeycombio/beeline-go

Legacy instrumentation for golang apps with Honeycomb

beeline-go's Issues

Debug mode is partially broken and does not print responses from the honeycomb API

I ran into this while trying to debug why some events were silently being dropped and not making it into honeycomb.

It seems that when you set Config{Debug: true} beeline starts a goroutine to read responses from libhoney transmission's response channel, so that they can be logged to stdout:

beeline-go/beeline.go

Lines 167 to 170 in a3ac12a

	if config.Debug {
	// TODO add more debugging than just the responses queue
	go readResponses(client.TxResponses())
	}

client.TxResponses() is a reference to the client package in this beeline module. It seems as though the package is setup to provide safe zero values if a client hasn't yet been configured, but the TxResponses() helper is broken - it always returns the safe zero value, even if the client has been configured (note the missing return on line 53):

beeline-go/client/client.go

Lines 51 to 59 in a3ac12a

	func TxResponses() chan transmission.Response {
	if client != nil {
	client.TxResponses()
	}
	c := make(chan transmission.Response)
	close(c)
	return c
	}

This means beeline.Init() function is receiving a closed channel, and never prints any of the responses to STDOUT.

One other thing that jumped out at me is that because the client variable is initialized with an empty libhoney.Client{}, the "safe zero value" behaviour for client.NewBuilder() is never called. If you change client to default to nil, the test suite panics at the point where it tries to send an event created from the zero value builder (line 16):

beeline-go/client/client_test.go

Lines 13 to 16 in a3ac12a

	b := NewBuilder()
	e := b.NewEvent()
	e.AddField("beep", "boop")
	e.Send()

propagation imports go.opentelemetry.io/otel/label which has been removed

$  go get -u ./...
# ...
	github.com/honeycombio/beeline-go/propagation imports
	go.opentelemetry.io/otel/label: cannot find module providing package go.opentelemetry.io/otel/label

Consider devirtualizing Timer

Currently Timer is an interface, and there's a comment saying:

// Represented
// as an interface so that other implementations can do fancy things with the
// values in addition to just returning them, such as submit them to
// instrumentation frameworks.

However that doesn't seem to be how the design of the code has evolved, timers are not responsible for submitting values.

I think it'd make sense to devirtualize this (i.e. change from an interface to a concrete struct), which should be good for performance.

Ovveride logger without replacing the libhoney client

Hello!
I was looking around in this library and I saw that libhoney accept a logger. But the libhoney.Client gets set during the beeline.Init. It means that in order to replace the Logger I need to override all Client. If I am reading it right. It does not sound friendly.

What if I add a Logger as part of the beeline.Config? In this way, it can be passed into the libhoney.Client.

Let me know if so I can do a PR.

Remove dependency on UUIDs

When we introduced trace context compatibility with W3C Trace Context headers, we went from using UUIDs to 16 byte arrays. It appears that we are still using UUIDs in db/sql and sqlx instrumentation to generate ids for transactions. If we used the same mechanism (random 8 or 16 byte hex encoded strings) for both we'd be able to get rid of the dependency on uuid.

Initializing the beeline with an empty writekey causes it to send events with a default

If you initialize a libhoney with an empty API key, it refuses to send any events, effectively turning the instrumentation into a noop. If you do the same with the beeline, it sends events to Honeycomb with a default API key. This is unexpected. Initializing the beeline with an empty API key should disable sending events to Honeycomb.

Improve gRPC wrapper support

It looks like Honeycomb recently added support for a unary server interceptor. I am interested in using Honeycomb to instrument some applications of mine that use gRPC, but they rely on client and server streaming.

It would be great if the hnygrpc package could implement the streaming server interceptor interface, as well as both the unary + streaming client interceptor interfaces.

Opentracing compliant tracer

Any plans for a tracer that implements the Opentracing API, as opposed to using Zipkin and having to spin up a proxy to send spans to Honeycomb?

Create a `RebindCtx(context.Context, string)` for the sql & sqlx wrappers

I'm guessing it's because the Rebind(string) function doesn't get a context, but right now it's creating floating spans that aren't attached to a trace.

https://github.com/honeycombio/beeline-go/blob/master/wrappers/hnysqlx/sqlx.go#L707

Allow modification of trace fields prior to serializing trace context

Our internal services propagate an identity token that represents the user/principal in our system that started the RPC call. Our systems add information from this token to the trace using AddFieldToTrace(), so that every span in the trace has that information.

When we're propagating trace context between processes there's little reason for us to include that information in the x-honeycomb-trace context as the upstream is going to re-extract it from the identity token and add it to its own part of the trace.

On internal GET requests/rpcs the identity token and honeycomb trace context accounts for a reasonable part of the payload, so it seems unnecessary to duplicate that information.

It would be good if we could specify a hook in the beeline that allows us to remove fields from the context prior to them being encoded, ala the PresendHook.

See slack discussion: https://honeycombpollinators.slack.com/archives/CLXQWT35J/p1574862771056800

Data race when sending spans

While running some tests with the race detector enabled, we noticed the following data race in the Span's Send method:

Read at 0x00c008aed051 by goroutine 173:
  github.com/{***}/vendor/github.com/honeycombio/beeline-go/trace.(*Span).Send()
      /go/src/github.com/{***}/vendor/github.com/honeycombio/beeline-go/trace/trace.go:241 +0x3f6
  github.com/{***}/vendor/github.com/{***}/finstrument.(*Span).Finish()

Previous write at 0x00c008aed051 by goroutine 212:
  github.com/{***}/vendor/github.com/honeycombio/beeline-go/trace.(*Span).Send()
      /go/src/github.com/{***}/vendor/github.com/honeycombio/beeline-go/trace/trace.go:250 +0x48c
  github.com/{***}/vendor/github.com/{***}/finstrument.(*Span).Finish()

I am using the latest master of libhoney-go (a6839afb6be6f3ec9de43b3a2c08f89b338e8534) and beeline-go (87bb646).

These lines correspond to the following code

• https://github.com/honeycombio/beeline-go/blob/master/trace/trace.go#L241
• https://github.com/honeycombio/beeline-go/blob/master/trace/trace.go#L250

It's not clear exactly what happened to me, but perhaps if a parent span is sent at the same time as one of its children it could lead to this race?

"database is closed" error originating from `hnysqlx`

seeing `error="sql: database is closed... goexit"

panic: runtime error: invalid memory address or nil pointer dereference
	panic: sync: negative WaitGroup counter
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x14c3f17]

goroutine 6937 [running]:
sync.(*WaitGroup).Add(0xc4226109a0, 0xffffffffffffffff)
	/usr/local/Cellar/go/1.10/libexec/src/sync/waitgroup.go:73 +0x133
sync.(*WaitGroup).Done(0xc4226109a0)
	/usr/local/Cellar/go/1.10/libexec/src/sync/waitgroup.go:98 +0x34
panic(0x18282e0, 0x1e11170)
	/usr/local/Cellar/go/1.10/libexec/src/runtime/panic.go:505 +0x229
github.com/honeycombio/hound/vendor/github.com/honeycombio/beeline-go/wrappers/hnysqlx.(*DB).NamedExec(0x0, 0x19557fd, 0x48, 0x18236e0, 0xc420c73770, 0x0, 0x0, 0x0, 0x0)
	/Users/cyen/src/go/src/github.com/honeycombio/hound/vendor/github.com/honeycombio/beeline-go/wrappers/hnysqlx/sqlx.go:345 +0x97```

panic: send on closed channel

Exact cause is unclear, and I am still investigating, but I'm intermittently seeing the following crash, which I think is caused by a race condition:

panic: send on closed channel

goroutine 2591 [running]:
github.com/honeycombio/libhoney-go/transmission.(*Honeycomb).Add(0xc0000ab040, 0xc0022fa000)
	/home/alex/go/pkg/mod/github.com/honeycombio/[email protected]/transmission/transmission.go:127 +0x21c
github.com/honeycombio/libhoney-go.(*Event).SendPresampled(0xc0041246e0, 0x0, 0x0)
	/home/alex/go/pkg/mod/github.com/honeycombio/[email protected]/libhoney.go:806 +0x366
github.com/honeycombio/beeline-go/trace.(*Span).send(0xc003640000)
	/home/alex/go/pkg/mod/github.com/alex/[email protected]/trace/trace.go:433 +0x372
github.com/honeycombio/beeline-go/trace.(*Span).sendLocked(0xc003640000)
	/home/alex/go/pkg/mod/github.com/alex/[email protected]/trace/trace.go:307 +0x301
github.com/honeycombio/beeline-go/trace.(*Span).Send(0xc003640000)
	/home/alex/go/pkg/mod/github.com/alex/[email protected]/trace/trace.go:252 +0x91
github.com/honeycombio/beeline-go/wrappers/common.BuildDBSpan.func1(0x0, 0x0)
	/home/alex/go/pkg/mod/github.com/alex/[email protected]/wrappers/common/common.go:203 +0xf8
github.com/honeycombio/beeline-go/wrappers/hnysqlx.(*Stmt).MustExecContext(0xc000735880, 0xf54f80, 0xc002be6090, 0xc002c6e600, 0x2f, 0x2f, 0xf4ce40, 0xc003725480)
	/home/alex/go/pkg/mod/github.com/alex/[email protected]/wrappers/hnysqlx/sqlx.go:1209 +0x1f0
[beyond this we're into my own code]

Propagation Header not being processed in Lambda APIs using HTTP Middleware

I am having trouble understanding why header propagation needs is happening only at the start of a trace, is this a requirement of the API or the data model?

beeline-go/wrappers/common/common.go

Lines 57 to 83 in fc19117

	func StartSpanOrTraceFromHTTPWithTraceParserHook(r *http.Request, parserHook config.HTTPTraceParserHook) (context.Context, *trace.Span) {
	ctx := r.Context()
	span := trace.GetSpanFromContext(ctx)
	if span == nil {
	// there is no trace yet. We should make one! and use the root span.
	var tr *trace.Trace
	if parserHook == nil {
	beelineHeader := r.Header.Get(propagation.TracePropagationHTTPHeader)
	ctx, tr = trace.NewTrace(ctx, beelineHeader)
	} else {
	// Call the provided TraceParserHook to get the propagation context
	// from the incoming request. This information will then be used when
	// create the new trace.
	prop := parserHook(r)
	ctx, tr = trace.NewTraceFromPropagationContext(ctx, prop)
	}
	span = tr.GetRootSpan()
	} else {
	// we had a parent! let's make a new child for this handler
	ctx, span = span.CreateChild(ctx)
	}
	// go get any common HTTP headers and attributes to add to the span
	for k, v := range GetRequestProps(r) {
	span.AddField(k, v)
	}
	return ctx, span
	}

My use case is that we have an AWS Lambda function, which accepts API calls and we establish the trace when the event hits the lambda, then we pass it through a layer which translates it into a HTTP request, and onto the honeycombe echo middleware.

When this middleware runs there is already a span active, so it doesn't read the propagation header.

If there is already a span running, can you link in the propagation from another source later?

Cheeres

Update for E&S

Is your feature request related to a problem? Please describe.

Environments & Services are coming to Honeycomb. The beeline should be updated to support sending data to environments while maintaining backward compatibility. To achieve this, we will set dataset from service name, and provide appropriate warnings depending on environment vs. non-environment teams.

Dataset should no longer propagate across services for new environment key.

Describe the solution you'd like

Tracing scenario for environment-aware teams:

• ServiceName "required"†. If unset, will warn and set service_name AND service.name to unknown_service:<process> or unknown_service:language. Service name is used to populate dataset name; if unknown_service.* then truncate tounknown_service for dataset name. service.name field will be added to the already added service_name for closer consistency with OTel.
• WriteKey required. If unset, will warn.
• Dataset ignored. If unset, no warning (because no longer used). If set, will warn to clarify that it will be ignored in favor of service name.

Tracing scenario for legacy/classic (non-environment-aware) teams:

• ServiceName "required"†. If unset, will warn.
• WriteKey required. If unset, will warn.
• Dataset required. If unset, will warn and default to beeline-<language>.

† While not technically required, service name is highly encouraged to avoid unknown_service and to instead properly describe the data being sent to Honeycomb (and the data being viewed in Honeycomb UI).

This will be addressed in a separate issue/PR: Set spanKind value based on OTel spec for consistency across beelines and OTel.

Tag for 0.3.1

Could we get a release tag for 0.3.1?

Enable Beeline to simultaneously accept w3c and honeycomb propagation headers

Is your feature request related to a problem? Please describe.

Currently, it's not possible to incrementally move a microservices architecture from the honeycomb propagation headers format to the w3c propagation headers format. This makes it difficult if not impossible for beeline customers with a large enough deployment to move towards interoperating with OpenTelemetry.

Describe the solution you'd like

Allow the beeline to simultaneously accept both w3c and honeycomb propagation headers, whichever is found on the incoming request. The outgoing propagation header format can still be static and set by configuration.

Describe alternatives you've considered

Additional context

Facebook patent grant questions

facebook code reused by the beeline library includes statements as a part of their patent grant an implicit termination of the license occurs should the user, or any affiliates be involved in ANY patent disputes with facebook.

Is the use of the facebook muster library a long term thing ? Implying that the patent rights are something any user of Honeycomb Go clients will need to surface to their legal team.

Beeline doesn't send events on a timer like libhoney's default

Observed: Using beeline.Init() with config defaults does not ever send events unless the batch size (50 by default) is met.

Expected: The default beeline config would also flush events on a 100ms timer like libhoney does by default.

I ran into this while developing on an endpoint locally that sends very few events; it seemed like the entire Honeycomb config was broken.

internal.ResponseWriter doesn't implement http.Hijacker

Hello! Thanks for the Go libraries – they're very useful.

In our API server, we create a gorilla.Mux that routes to both HTTP endpoints and Websocket endpoints. The Websocket endpoints use a websocket.Upgrader.

Unfortunately, Beeline's internal.ResponseWriter doesn't implement http.Hijacker, which the websocket upgrader requires. So we can't use Beeline for those endpoints.

We're working around the issue by only piping the HTTP handlers through the supplied Gorilla middleware, but it would be great if it worked (or at least didn't break) our websocket endpoints.

We've previously used https://github.com/felixge/httpsnoop to do this for our websocket endpoints, and it looks like you used to also. Though I understand why you might want to reduce your number of third-party dependencies.

Thanks again!

hnygorilla post-handler fields

hnynethttp, post-handler, sets:

• response.status_code
• response.content_length
• response.content_type
• response.content_encoding

of these, hnygorilla only sets status_code

Should hnygorilla set the others as well? And/or perhaps these fields should be abstracted into common.NewResponseWriter?

The SQL wrapper should not call libhoney's singleton, package-level functions

Tangentially related to #55. When we upgraded to the latest master of beeline-go some of our apps stopped reporting SQL events:

It seems that only the functions that don't accept a context.Context stopped reporting. I think this is because the wrapper is calling singleton methods on libhoney-go, but beeline-go no longer initializes libhoney-go's global Client, so the events are being silently dropped. Maybe the wrappers should use github.com/honeycombio/beeline-go/client instead?

In the meantime I'm taking this as a kick to change our app to use the context-ful variant 😅.

Improve documentation for `trace` package

In the doc strings for the functions creating a new trace (https://godoc.org/github.com/honeycombio/beeline-go/trace#NewTrace and the following two) it never states that the returned trace has an active span. This leads to the possibly incorrect interpretation that one would call NewTrace() and then immediately after call StartSpan in order to open your first span. The documentation could be improved here by calling out that once a trace is started you have an active span and need only create new ones for surrounding smaller units of work.

Stitching spans together

Hi, I have an asynchronous job processing system that uses this honeycomb package. I'd like to be able to tie all the spans from job enqueue to job process to one trace. Is there a recommended way to do that? I was thinking in the job processing I can create a new span with the parent ID and trace ID from the job enqueueing process span, but was hoping to get some thoughts before going down that route.

Thank you in advance.

echo example creates a binary that shadows sh `echo` builtin

Please consider renaming the directory that the echo example is in. As it is, experimenting with beeline-go caused $GOPATH/bin/echo to shadow /bin/echo in totally unrelated configure scripts & makefiles. The behavior was extremely surprising and confusing.

The offending file:
https://github.com/honeycombio/beeline-go/blob/9adb5c235b865f43dd103e11ceb5d45ceb05c4e1/examples/echo/main.go

I originally reported this, erroneously, as an issue with the Echo framework. More details over there: labstack/echo#1408

Memory usage is high for long (duration) traces

We are using tracing to understand the lifecycle of connections that can be very long lived. The way the trace package is designed, spans continue to be referenced by the root span until the root span is sent. As a result, spans end up consuming memory even after they have been sent.

The net/http Transport wrapper should allow you to attach extra metadata to spans

There are some situations where it's helpful to record extra, arbitrary information about http requests and responses to the spans created by the net/http Transport wrapper.

In some of our systems we've found that third party APIs often return incredibly useful metadata in HTTP response headers, e.g.:

• the version of their API that serviced the request
• region that serviced the HTTP request
• deprecation warnings about API usage
• details of rate limit consumption/allowance
• some of our internal apps return the route pattern/shape they used to match the request, which makes it easier to group HTTP calls made by a http client

To implement this right now you'd have to create another roundtripper that sits between the honeycomb transport and the default net/http.Client{}.Transport, which seems unnecessarily awkward/adds yet more wrappers to net/http.Client{}.Transport.

It'd be nice if the beeline transport offered an explicit way to extract and add this metadata, e.g.:

hnynethttp.WrapRoundTripperWithMetadataHook(http.RoundTripper, func(http.Request, *http.Response, error) map[string]interface{})

I guess prior art for this would be the PresendHook on the Config struct?

sqlx package uses json tags instead of db tags when serializing objects

If you have a struct type that you pass in to one of the sqlx methods, and this struct type has separate json and db tags (or only json tags), the json tags will be honored when sending those objects to Honeycomb as db.query.args instead of the db tags. This can be confusing when specific members of the struct are omitted from json encoding but expected by the database (such as ID fields). They will appear absent in Honeycomb despite having been correctly passed to the DB.

Example:

type Foo struct {
	ID                 int            `json:"id"`
	Name           string       `json:"-"`
	Color            string       `json:"color"`
}

f := Foo{1, "wibbly", "green"}
db.MustExec("INSERT into foo VALUES (:id, :name, :color)", f)

The DB statement will successfully run that INSERT, but the db.query.args that shows up in Honeycomb will be [{"id":1,"color":"green"}]

hnysql wrapping strategy limits interoperability

The hysql.DB type does not embed a *sql.DB, and therefore is a not a drop-in replacement. This means users of hnysql are not easily able to integrate with any libraries or code that expects a type of *sql.DB.

The comment says

	// wdb is the wrapped sql db. It is not embedded because it's better to fail
	// compilation if some methods are missing than it is to silently not
	// instrument those methods. If you believe that this wraps all methods, it
	// would be reasonable to think that calls that don't show up in Honeycomb
	// aren't happening when they are - they just fell through to the underlying
	// *sql.DB. So all methods present on *sql.DB are recreated here, but as the
	// wrapped package changes, we will fail to compile against apps using those
	// new features and need a patch.

I understand the motivation here, but it does severely limit the usability of this package. For example, the popular sqlx package embeds the stdlib type, and therefore can be used as a drop-in replacement.

Opening this issue to request an implementation that does embed the stdlib type so that we can integrate with existing code easily.

Consider adding HTTP referrer?

Currently, the Go Beeline doesn't capture http referrer. That might be an interesting case -- would it be worth adding the 'Referrer' header field? (I can imagine some people might have privacy issues)

DeterministicSamplerTest flakey

I noticed that the DeterministicSamplerTest fails occasionally on dependabot PRs (for example: https://app.circleci.com/pipelines/github/honeycombio/beeline-go/958/workflows/37be948a-6bb5-4606-a629-e5c6f785e07e/jobs/2992)

Re-running passes

Please downgrade go-sqlite3

Versions

• Go: 1.19
• Beeline: 1.9.0

Steps to resolve

1. Remove this line from go.mod:

   github.com/mattn/go-sqlite3 v2.0.3+incompatible // indirect
   

2. Run go mod tidy

Additional context

mattn/go-sqlite3#975

TL;DR: Version 2 of go-sqlite3 is unstable

Separate client-span and trace-propagation concerns

Is your feature request related to a problem? Please describe.

Currently, the http and grpc client wrappers perform two main functions:

1. They create new spans, to which information like request endpoints, response codes, and headers can be attached
2. They propagate the trace/span metadata across the connection to their corresponding server wrappers

These two functions are coupled; you can't get one without the other.

However, the creation and sending of client spans can (especially in a microservice architecture) nearly double the number of spans sent as part of a trace. The information contained in a client span -- headers, endpoint, etc -- is largely duplicated in the span sent by the server wrapper. This means that a team seeking to reduce its event volume might want to remove client spans from the equation, to get an easy 50% reduction in volume with little loss in data.

Describe the solution you'd like

A separation of concerns in the client wrappers. If appropriately configured, they should perform either (1), or (2), or both, as the user wants.

Describe alternatives you've considered

We tried using our sampler to simply discard client spans. However, because they're still created and have their IDs sent across the network, the corresponding server spans all have no parent. This breaks the ability of the honeycomb UI to infer causality.

HoneyComb HTTP wrapper crashes with a value HTTP Handler

Tested using v1.1.2:

package main

import (
	"net/http"
	"net/http/httptest"

	"github.com/honeycombio/beeline-go/wrappers/hnynethttp"
)

type plainHandler struct{}

func (h plainHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}

func main() {
	mx := http.NewServeMux()
	mx.Handle("/h", plainHandler{})

	beeline.Init(beeline.Config{STDOUT: true})

	w := httptest.NewRecorder()
	r := httptest.NewRequest("POST", "/h", nil)

	hnynethttp.WrapHandler(mx).ServeHTTP(w, r)
}

Running this program crashes (panic):

reflect.Value.Pointer(0x131a280, 0x15c26e8, 0x99, 0x15c26e8)
/usr/local/go/src/reflect/value.go:1489 +0x1ae
github.com/honeycombio/beeline-go/wrappers/hnynethttp.WrapHandlerWithConfig.func1(0x13d6fd0, 0xc000152480, 0xc00020e100)
[email protected]/wrappers/hnynethttp/nethttp.go:45 +0x630
net/http.HandlerFunc.ServeHTTP(0xc000115200, 0x13d6fd0, 0xc000152480, 0xc00020e100)
/usr/local/go/src/net/http/server.go:2069 +0x44
main.main()
muxxy-bug/main.go:25 +0x1d3
exit status 2

This is linked with the way the name of the underlying handler is found, which assumes that this is always a pointer:

handler, pat := mux.Handler(r)
name := runtime.FuncForPC(reflect.ValueOf(handler).Pointer()).Name()

A possible workaround is to make the receiver a pointer value (but this is a workaround, heh!)
func (h *plainHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { }

hnysqlx doesn't wrap Select (with Queryer) or Ext

I'm early in this process so I've not yet found a workaround for the way using these from sqlx with other stuff from hnysqlx doesn't seem to play nice:

func Select(q Queryer, dest interface{}, query string, args ...interface{}) error

https://godoc.org/github.com/jmoiron/sqlx#Select

type Ext

https://godoc.org/github.com/jmoiron/sqlx#Ext

Does anyone have any thoughts?

perf: "app." + key is allocating new objects still

Versions

• Go: 1.21.4
• Beeline: 1.13.0

Steps to reproduce

1. Run shepherd at high volume ;)
2. Look at the profiles.
3. Weep.

Additional context
#328 tried to make this better (before, we were using sprintf which is worse ugh), but it's now 7% of our allocations. This is not good. We probably would save time using a LRU cache for mapping foo to app.foo to avoid allocating brand new strings every time.

Rebind instrumentation missing ctx

From Slack:

In the beagle team we were discussing the presence (and uselessness) of an enormous number of Rebind calls that are not part of any trace. I think this is coming from the beeline sqlx wrapper. I’m not sure how we would approach this, but the spans represent string manipulation (so are never slow) and might be useful if they were part of a trace but there’s no ctx being passed in, so they’re just spewing garbage.

hnysqlx: missing fields when using existing prepared statements within a transaction

Hi!

When using the honeycomb sqlx wrapper, and using an existing prepared statement from a transaction, it seems the prepared statement tracing fields are lost (i.e., db.query).

After digging a bit in the implementation of the honeycomb sqlx wrapper it seems the reason for this to happen is that the fields are added to the prepared statement object builder:
https://github.com/honeycombio/beeline-go/blob/master/wrappers/hnysqlx/sqlx.go#L539

But when calling a prepared statement from a transaction, the transaction builder is used instead (which lacks fields such as the db.query):
https://github.com/honeycombio/beeline-go/blob/master/wrappers/hnysqlx/sqlx.go#L1824-L1828

This would mean the original prepared statement fields are lost from that point on.

Here's a simplified code snippet illustrating the issue:

struct object {
     ID string `db:"id"`
     UpdatedAt time `db:"updated_at"`
}

func reproduceIssue() error {
	db, err := sqlx.Open("postgres", uri)
	if err != nil {
		return err
	}

	wdb := hnysqlx.WrapDB(db)

	query := "UPDATE my_table SET updated_at = now() where id = :id RETURNING updated_at"
	// prepare a statement 
	if updateStmt, err = wdb.PrepareNamed(query)); err != nil {
		return err
	}

	o := &object{ ID: "id-1" }

       // This query will have a `db.query` field in the trace
	if err := updateStmt.QueryRowContext(ctx, o).Scan(&o.UpdatedAt); err != nil {
		return err
	}

	// open a transaction that uses the prepared statement
	tx, err := wdb.BeginTxx(context.Background(), nil)
	if err != nil {
		return err
	}

	// This query won't have a `db.query` field in the trace
	if err := tx.NamedStmt(updateStmt).QueryRowContext(ctx, o).Scan(&o.UpdatedAt); err != nil {
		tx.Rollback()
		return err
	}

	return tx.Commit()
}

Note: the scenario where I've encountered the issue is with NamedStmt, however I think it might also apply to Stmt. I found it suspicious that the db.query field isn't added to the db.Preparex call when it is done for the db.PrepareNamed.
https://github.com/honeycombio/beeline-go/blob/master/wrappers/hnysqlx/sqlx.go#L577

Also, this seems to be handled properly for the cases where a prepared statement is created from a transaction, but not when an existing prepared statement is used from a transaction.
https://github.com/honeycombio/beeline-go/blob/master/wrappers/hnysqlx/sqlx.go#L1884

Hope this is clear!

Thanks in advance,
Esther

Expose Version as a public field on beeline

There should be a straightforward way to programmatically extract Version from beeline.

Consider option to specify which field value should go in 'name'?

In the instrumentation for sqlx name gets populated by method names like ExecContext which is OK but it might be nice to be able to specify that a field like db.query is used instead.

Then you would see SELECT * FROM ... in the name column and reasoning about sequences of queries would be easier. Or, maybe it would be nice to somehow have a dataset level setting for which fields should be unrolled by default and set it there. Not sure how I'd approach this one.

invalid memory address or nil pointer dereference when sending spans

I'm getting a (very rare) panic when sending spans. We're creating an async span in a separate goroutine right around the same time that we send the parent span. I think this creates some contention around the span.children. See the below stack trace.

Oct 31 16:21:48 ip-10-10-3-60 d51df3 syslog ERROR ERROR: 2018/10/31 23:21:47 recovery.go:53: PANIC: runtime error: invalid memory address or nil pointer dereference
Oct 31 16:21:48 ip-10-10-3-60 d51df3 syslog  goroutine 74281 [running]:
Oct 31 16:21:48 ip-10-10-3-60 d51df3 syslog  github.com/launchdarkly/event-recorder/vendor/github.com/launchdarkly/foundation/middleware.(*Recovery).ServeHTTP.func1(0x1a7e1c0, 0xc4956dbce0, 0xc458fb5920, 0x0, 0x0, 0xc53bd3b500, 0xc51eaac300, 0x2cd, 0x6ce)
Oct 31 16:21:48 ip-10-10-3-60 d51df3 syslog  #011/go/src/github.com/launchdarkly/event-recorder/vendor/github.com/launchdarkly/foundation/middleware/recovery.go:42 +0xf5
Oct 31 16:21:48 ip-10-10-3-60 d51df3 syslog  panic(0x10bf060, 0x1abaa20)
Oct 31 16:21:48 ip-10-10-3-60 d51df3 syslog  #011/usr/local/go/src/runtime/panic.go:491 +0x283
Oct 31 16:21:48 ip-10-10-3-60 d51df3 syslog  github.com/launchdarkly/event-recorder/vendor/github.com/honeycombio/beeline-go/trace.(*Span).IsAsync(...)
Oct 31 16:21:48 ip-10-10-3-60 d51df3 syslog  #011/go/src/github.com/launchdarkly/event-recorder/vendor/github.com/honeycombio/beeline-go/trace/trace.go:252
Oct 31 16:21:48 ip-10-10-3-60 d51df3 syslog  github.com/launchdarkly/event-recorder/vendor/github.com/honeycombio/beeline-go/trace.(*Span).Send(0xc49d23e080)
Oct 31 16:21:48 ip-10-10-3-60 d51df3 syslog  #011/go/src/github.com/launchdarkly/event-recorder/vendor/github.com/honeycombio/beeline-go/trace/trace.go:238 +0x263

Race in sampler hook

When we switched to the new tracing stuff introduced in v0.2.x, we started seeing panics like the following.

Oct 25 10:27:49 ip-10-10-1-212 b94ce8 syslog  goroutine 64181 [running]:
Oct 25 10:27:49 ip-10-10-1-212 b94ce8 syslog  runtime.throw(0x12ac57f, 0x26)
Oct 25 10:27:49 ip-10-10-1-212 b94ce8 syslog  #011/usr/local/go/src/runtime/panic.go:605 +0x95 fp=0xc51d7eee90 sp=0xc51d7eee70 pc=0x42dc45
Oct 25 10:27:49 ip-10-10-1-212 b94ce8 syslog  runtime.mapiternext(0xc51d7ef030)
Oct 25 10:27:49 ip-10-10-1-212 b94ce8 syslog  #011/usr/local/go/src/runtime/hashmap.go:778 +0x6f1 fp=0xc51d7eef28 sp=0xc51d7eee90 pc=0x40bad1
Oct 25 10:27:49 ip-10-10-1-212 b94ce8 syslog  github.com/launchdarkly/event-recorder/vendor/github.com/launchdarkly/foundation/honey.SamplerHook(0xc54581ffb0, 0x12901de, 0xe)
Oct 25 10:27:49 ip-10-10-1-212 b94ce8 syslog  #011/go/src/github.com/launchdarkly/event-recorder/vendor/github.com/launchdarkly/foundation/honey/honeycomb.go:35 +0x127 fp=0xc51d7ef130 sp=0xc51d7eef28 pc=0xecfc17
Oct 25 10:27:49 ip-10-10-1-212 b94ce8 syslog  github.com/launchdarkly/event-recorder/vendor/github.com/honeycombio/beeline-go/trace.(*Span).send(0xc4896de500)
Oct 25 10:27:49 ip-10-10-1-212 b94ce8 syslog  #011/go/src/github.com/launchdarkly/event-recorder/vendor/github.com/honeycombio/beeline-go/trace/trace.go:328 +0x3c6 fp=0xc51d7ef278 sp=0xc51d7ef130 pc=0x753f66
Oct 25 10:27:49 ip-10-10-1-212 b94ce8 syslog  github.com/launchdarkly/event-recorder/vendor/github.com/honeycombio/beeline-go/trace.(*Span).Send(0xc4896de500)
Oct 25 10:27:49 ip-10-10-1-212 b94ce8 syslog  #011/go/src/github.com/launchdarkly/event-recorder/vendor/github.com/honeycombio/beeline-go/trace/trace.go:229 +0x2df fp=0xc51d7ef358 sp=0xc51d7ef278 pc=0x75366f
Oct 25 10:27:49 ip-10-10-1-212 b94ce8 syslog  github.com/launchdarkly/event-recorder/vendor/github.com/honeycombio/beeline-go/trace.(*Span).Send(0xc4896de200)
Oct 25 10:27:49 ip-10-10-1-212 b94ce8 syslog  #011/go/src/github.com/launchdarkly/event-recorder/vendor/github.com/honeycombio/beeline-go/trace/trace.go:224 +0x2ba fp=0xc51d7ef438 sp=0xc51d7ef358 pc=0x75364a
...truncated

Presumably, we're adding fields in another routine at the same time we're trying to send the span. That's clearly not good and I'm working to identify where we're doing this. However, I think it would be better if that error didn't cause a panic. Maybe send could acquire the same lock that is used when AddField is called?

go.sum entries not aligned

Hey team,

It seems that in v1.1.0 go.sum needs updating.

[0:~/wf/other> git clone https://github.com/honeycombio/beeline-go

[0:wf/other/beeline-go (main)> git checkout v1.1.0                           
Note: switching to 'v1.1.0'.

[0:wf/other/beeline-go (tags/v1.1.0)> go build           
go: github.com/gin-gonic/[email protected] requires
	github.com/go-playground/validator/[email protected] requires
	golang.org/x/[email protected]: missing go.sum entry; to add it:
	go mod download golang.org/x/crypto

It would probably make sense to add go build to the CI, so that kind of stuff would be caught on CI stage.

GRPC wrapper does not appear to capture standard request headers

Is your feature request related to a problem? Please describe.

Spans for wrapper GRPC request handler functions don't have common request headers included.

Describe the solution you'd like

Specifically, x-forwarded-for if it is available, but consider what other headers appear commonly on GRPC requests.

Prior art:

• our HTTP wrapper's use of GetRequestProps
• OTel's GRPC instrumentation?

Error when running go get -u github.com/honeycombio/beeline-go/..

New to honeycomb.io.

Running go get -u github.com/honeycombio/beeline-go/...

`go: github.com/go-playground/validator/v10 upgrade => v10.4.1
go: github.com/gobuffalo/validate/v3 upgrade => v3.3.0
go: github.com/Masterminds/semver/v3 upgrade => v3.1.0
go: github.com/jackc/pgproto3/v2 upgrade => v2.0.5
go: github.com/mattn/go-colorable upgrade => v0.1.8
go: github.com/modern-go/reflect2 upgrade => v1.0.1
go: github.com/gobuffalo/packd upgrade => v1.0.0
go: github.com/json-iterator/go upgrade => v1.1.10
go: github.com/gofrs/uuid upgrade => v3.3.0+incompatible
go: github.com/golang/protobuf upgrade => v1.4.3
go: github.com/gobuffalo/fizz upgrade => v1.13.0
go: github.com/gobuffalo/nulls upgrade => v0.4.0
go: github.com/jackc/chunkreader/v2 upgrade => v2.0.1
go: golang.org/x/sys upgrade => v0.0.0-20201026173827-119d4633e4d1
go: go.opentelemetry.io/contrib/propagators upgrade => v0.13.0
go: gopkg.in/yaml.v2 upgrade => v2.3.0
go: github.com/mattn/go-isatty upgrade => v0.0.12
go: github.com/gobuffalo/flect upgrade => v0.2.2
go: google.golang.org/protobuf upgrade => v1.25.0
go: golang.org/x/crypto upgrade => v0.0.0-20201016220609-9e8e0b390897
go: golang.org/x/sync upgrade => v0.0.0-20201020160332-67f06af15bc9
go: github.com/gobuffalo/helpers upgrade => v0.6.1
go: golang.org/x/text upgrade => v0.3.3
go: github.com/ugorji/go/codec upgrade => v1.1.13
go: github.com/gobuffalo/pop/v5 upgrade => v5.3.1
go: github.com/rogpeppe/go-internal upgrade => v1.6.2
go: github.com/jackc/pgservicefile upgrade => v0.0.0-20200714003250-2b9c44734f2b
go: golang.org/x/net upgrade => v0.0.0-20201026091529-146b70c837a4
go: github.com/modern-go/concurrent upgrade => v0.0.0-20180306012644-bacd9c7ef1dd
go: github.com/pkg/errors upgrade => v0.9.1
go: github.com/jackc/pgconn upgrade => v1.7.0
go: github.com/mattn/go-sqlite3 upgrade => v1.14.4
go: github.com/vmihailenco/tagparser upgrade => v0.1.2
go: github.com/jackc/pgtype upgrade => v1.5.0
go: github.com/sergi/go-diff upgrade => v1.1.0
go: go.opentelemetry.io/otel upgrade => v0.13.0
go: golang.org/x/xerrors upgrade => v0.0.0-20200804184101-5ec99f83aff1
go: github.com/gobuffalo/envy upgrade => v1.9.0
go: google.golang.org/appengine upgrade => v1.6.7
go: github.com/microcosm-cc/bluemonday upgrade => v1.0.4
go: github.com/jackc/pgx/v4 upgrade => v4.9.0
go: github.com/go-sql-driver/mysql upgrade => v1.5.0
go: github.com/julienschmidt/httprouter upgrade => v1.3.0
go: github.com/valyala/fasttemplate upgrade => v1.2.1
go: github.com/gobuffalo/tags/v3 upgrade => v3.1.0
go: github.com/klauspost/compress upgrade => v1.11.1

github.com/honeycombio/beeline-go/propagation

../../../go/pkg/mod/github.com/honeycombio/[email protected]/propagation/b3.go:33:32: propagator.GetAllKeys undefined (type b3.B3 has no field or method GetAllKeys)
../../../go/pkg/mod/github.com/honeycombio/[email protected]/propagation/w3c.go:31:16: undefined: propagators.DefaultHTTPPropagator
../../../go/pkg/mod/github.com/honeycombio/[email protected]/propagation/w3c.go:55:16: undefined: propagators.DefaultHTTPPropagator
`

This prevents the app from building or running

Introduction of libhoney.Client should be documented in changelog

This commit changed beeline.Init() so that it doesn't call the global libhoney.Init. This broke some of our services that are using both libhoney events and beeline traces while they migrate to libhoney.

Please can you document this change in the changelog?

upgrade pop to v6

pop v5 has transitive dependencies (bluemonday) that have known vulnerabilities. upgrade to v6

we'll need to drop go 1.14 from support

(from a cursory look, it seems they still have not upgraded all the things, so we may still see an alert, but at least now dependabot can keep us up to date on v6)

Libhoney events sent to honeycomb API return "x509: certificate signed by unknown authority" error.

MacOS Version: High Sierra 10.13.6
Golang Version: go1.12.6 darwin/amd64
Libhoney Version: v1.12.4
Client Docker Engine:

• Version: 19.03.2
• API version: 1.40
• Go version: go1.12.8
• OS/Arch: darwin/amd64
• Experimental: false

I'm trying to send an event to my dataset using the libhoney-go package. I'm following the first example at: https://docs.honeycomb.io/getting-data-in/go/sdk/#examples.

Here is a snippet of my code which is running in a local docker container

func main() {
	libhoney.Init(libhoney.Config{
		WriteKey: "KEY",
		Dataset: "DATASET",
		
	})
		
	defer libhoney.Close() // Flush any pending calls to Honeycomb
	
	ev := libhoney.NewEvent()
	ev.Add(map[string]interface{}{
		"duration_ms": 153.12,
		"method": "get",
		"hostname": "appserver15",
		"payload_length": 27,
	})

	log.Println("Sending event...")
	log.Println(ev)
		  
	ev.Send()
	c := libhoney.Responses()

	log.Println("Waiting to receive response from honeycomb...")
	res := <-c
	log.Println(res)
}

The response returned from the libhoney.Responses() channel is:

x509: certificate signed by unknown authority

A lot of the docs online point to this being an issue with MacOS and Docker (https://forums.docker.com/t/docker-private-registry-x509-certificate-signed-by-unknown-authority/21262) but I've played around with my keychains and can't overcome this issue. I suspect keychains aren't the issue because I'm running the nodejs beeline package in a different docker container and it has no issues sending messages to Honeycomb.

I was wondering if something like this has come up previously with a different user? Any help would be appreciated, thanks :)

Tag for version v1.1.3 has a typo.

The tag should be v1.1.3 but it is v.1.1.3.

Took me a while to figure out why I could not go get at 1.1.3.

Echo middleware incompatible with v4

The latest version of Echo requires the you explicitly import the module using the /v4 prefix:

Any of these versions will allow you to import Echo as github.com/labstack/echo/v4 which is the recommended way of using Echo going forward.

So the current Echo middleware is usable only with version 3:

./main.go:56:33: cannot use hnyecho.New().Middleware() (type "github.com/labstack/echo".MiddlewareFunc) as type "github.com/labstack/echo/v4".MiddlewareFunc in argument to e.Use

In the meantime I'm able to work around this issue by duplicating the code from the middleware into my own middleware that imports /v4