-
Notifications
You must be signed in to change notification settings - Fork 31
Support Lambda Platform Metrics #202
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
Conversation
This commit is heavily inspired by the Go agent codebase. The data model needed to (un)marshal metrics as well as the associated metadata is defined.
This file contains information about the extension version. This used to enrich Metrics metadata with the version of the extension used to generate these metrics.
This commit allows for the extraction of the metadata from the first received transaction, with the assumption that this metadata will persist across invocations between cold starts. Metadata is obtained from the uncompressed payload, and a dedicated container is populated.
These changes allow for populating dedicated structs by using the Lambda platform report provided after each invocation. Some fields also need to be added to the Event structs. A notable addition is the modification of the metadata to incorporate extension-specific fields. I think that this information should be included, but the current implementation causes the extension to be displayed as a separate service in the APM UI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good, mostly minor comments.
apm-lambda-extension/go.mod
Outdated
@@ -15,9 +15,15 @@ require ( | |||
gotest.tools v2.2.0+incompatible | |||
) | |||
|
|||
require ( | |||
go.elastic.co/apm v1.15.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use go.elastic.co/apm/v2
, the current major version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one thing (we should use Go agent v2).
I assume we'll want the labels to be recorded as faas.*
, rather than labels.faas_*
, right? Is your plan that once the server is updated, we'll come back and update the Go agent's model and the extension to send them as metricset fields? Similar to what we do for transaction and span properties: https://github.com/elastic/apm-agent-go/blob/94d06659448a555142c087721a79fff3ee732b49/model/model.go#L798-L804
@axw Thank you for the review. I have updated the model to |
Motivation / Summary
This PR is the continuation of #127, which had become obsolete and non-digestible due to the many changes brought to the extension since the initialization of the PR. The current PR implements the same features, but was rebased to enable a commit-by-commit review approach. It resolves #108.
The general flow is as follows:
SubEventType
alongsideRuntimeDone
:Report
. This report contains the metrics that we want to send to the APM Server. These metrics are packaged inside an APM payload (alongside the extracted metadata) in a dedicated function.Files / Added Elements
model
which is a partial reproduction of the data model used by the Go APM Agentextension
package. Metadata is only extracted from the first APM payload received during the current lifetime of the extension.logsapi
package.main
updates to modify the goroutines in order to extract metadata and process metrics.Limitations / Elements to discuss
i
will be received during the execution of the invocationi+1
.apm-lambda-extension
when the metadata is attached to the generated metrics. While this makes sense in the framework of observability, the current APM UI is not equipped accordingly, and the service is associated to an unknown runtime:My suggestion would be to stop altering the metadata for now to provide a consistent experience to end users.
logsapi
andextension
completely separate, but in the current implementation,logsapi.ProcessLogs()
takes anapmServerTransport
in argument in order to enqueue APM metrics payloads.faas
fields will be added to themetricset
object in the APM data model (PR to be created). The subsequent modifications are beyond the scope of this initial PR, which aims to provide an initial implementation of Lambda metrics generation.How to test