[PoC] Register partial transaction in AWS extn #1352
Conversation
module/apmlambda/lambda.go
Outdated
| } else { | ||
| resp, err := http.Post( | ||
| // TODO: @lahsivjar better way to get base URI | ||
| "http://localhost:8200/register/transaction", |
There was a problem hiding this comment.
@axw I wanted to keep the endpoints local to the lambda module. In the agent config I see that we have option for multiple APM server URLs so I am not sure what is the best way to get the base URI (I think it would always be localhost:8200 but hard coding like this is probably not the best idea)
There was a problem hiding this comment.
Hmm, I'm not sure what the best approach is here. Since it's a special case, I also wouldn't want to complicate the main API.
ELASTIC_APM_SERVER_URLS (multiple URLs) is deprecated, and I think the only time you would set a non-default URL is for testing purposes. So I'd say just use os.Getenv("ELASTIC_APM_SERVER_URL") here, and default to http://localhost:8200.
There was a problem hiding this comment.
@lahsivjar thanks, I think this turned out pretty well. We could neaten it up a bit if we want to go ahead with it, but I think it's proven the approach can work.
| @@ -258,6 +225,40 @@ func (w *modelWriter) buildModelError(out *model.Error, e *ErrorData) { | |||
| out.Culprit = truncateString(out.Culprit) | |||
| } | |||
|
|
|||
| // BuildModelTransaction converts apm transaction to model transaction | |||
There was a problem hiding this comment.
I don't love exposing this just for Lambda. td.Context.build() mutates the transaction context, for one thing. I think it's okay as it is, but could be a source of bugs later down the line.
Maybe we could have a method which only copies immutable properties (IDs, sampled status, sample rate) to the model object? Or otherwise, perhaps we could expose those properties as methods on Transaction, and build the partial transaction model directly in module/apmlambda code. Would that be awful?
lahsivjar
changed the title
lahsivjar
force-pushed
the
1323-lambdatxn
branch
from
5509f85
to
896ddd4
Compare
💚 Build Succeeded
Expand to view the summary
Build stats
🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
Register the transaction with lambda extension to handle unexpected failures and crashes. In these unexpected cases, the lambda extension will create a proxy transaction on behalf of the agent.
Closes: #1323