-
Notifications
You must be signed in to change notification settings - Fork 236
feat(collector/receiver): support extra fields in structured JSON function logs #2251
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
base: main
Are you sure you want to change the base?
Changes from all commits
34b1730
56f6141
a211735
d9c75a0
1ad4f56
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -490,6 +490,19 @@ func (r *telemetryAPIReceiver) createLogs(slice []event) (plog.Logs, error) { | |||||
| } | ||||||
| } else if line, ok := record["message"].(string); ok { | ||||||
| logRecord.Body().SetStr(line) | ||||||
|
|
||||||
| for key, value := range record { | ||||||
| switch key { | ||||||
| case "level", "message", "requestId", "timestamp": | ||||||
| continue | ||||||
| default: | ||||||
| attr, _ := logRecord.Attributes().GetOrPutEmpty(key) | ||||||
| if err := attr.FromRaw(value); err != nil { | ||||||
| r.logger.Warn(fmt.Sprintf("error creating attribute: %s", key), zap.Error(err)) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I personally do not feel that after emiting an error level process should continue and that's why I chose warning here. Probably it is more preferred to prioritize handling request correctly instead stopping it due to malformed log metadata. Warning developer that something was not correctly logged should be enough IMHO. Anyway agree on the zap usage. |
||||||
| continue | ||||||
| } | ||||||
|
Comment on lines
+500
to
+503
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If an error is returned out of the |
||||||
| } | ||||||
| } | ||||||
| } | ||||||
| } else { | ||||||
| if requestId := r.getCurrentRequestId(); requestId != "" { | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -359,6 +359,7 @@ func TestCreateLogs(t *testing.T) { | |
| containsRequestId bool | ||
| requestId string | ||
| severityNumber plog.SeverityNumber | ||
| attributes map[string]interface{} | ||
| } | ||
|
|
||
| testCases := []struct { | ||
|
|
@@ -479,6 +480,109 @@ func TestCreateLogs(t *testing.T) { | |
| }, | ||
| expectError: false, | ||
| }, | ||
| { | ||
| desc: "function json with extra fields", | ||
| slice: []event{ | ||
| { | ||
| Time: "2026-02-26T20:15:32.000Z", | ||
| Type: "function", | ||
| Record: map[string]any{ | ||
| "timestamp": "2026-02-26T20:15:32.000Z", | ||
| "level": "INFO", | ||
| "requestId": "79b4f56e-95b1-4643-9700-2807f4e6", | ||
| "message": "Hello world, I am a function with extra data!", | ||
| "extraString": "stringValue", | ||
| "extraNumber": 2217, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can never happen in production. a Record is always populated via |
||
| "extraFloat": 3.14, | ||
| "extraBoolean": true, | ||
| "extraNull": nil, | ||
| "extraArrayOfStrings": []any{"stringValue", "stringValue"}, | ||
| "extraArrayOfNumbers": []any{2217, 2217}, | ||
| "extraArrayOfMixedTypes": []any{"stringValue", 2217, true, nil}, | ||
| "extraArrayWithNesting": []any{"stringValue", []any{2217, []any{true, nil}}}, | ||
| "extraObject": map[string]any{ | ||
| "stringValue": "stringValue", | ||
| "numberValue": 2217, | ||
| "booleanValue": true, | ||
| "nullValue": nil, | ||
| }, | ||
| "extraObjectWithNesting": map[string]any{ | ||
| "stringValue": "stringValue", | ||
| "numberValue": 2217, | ||
| "booleanValue": true, | ||
| "nullValue": nil, | ||
| "arrayValue": []any{"stringValue", 2217}, | ||
| "objectValue": map[string]any{ | ||
| "stringValue": "stringValue", | ||
| "numberValue": 2217, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| expectedLogs: []logInfo{ | ||
| { | ||
| logType: "function", | ||
| timestamp: "2026-02-26T20:15:32.000Z", | ||
| body: "Hello world, I am a function with extra data!", | ||
| containsRequestId: true, | ||
| requestId: "79b4f56e-95b1-4643-9700-2807f4e6", | ||
| severityText: "Info", | ||
| severityNumber: plog.SeverityNumberInfo, | ||
| attributes: map[string]any{ | ||
| "extraString": "stringValue", | ||
| "extraNumber": int64(2217), | ||
| "extraFloat": float64(3.14), | ||
| "extraBoolean": true, | ||
| "extraNull": nil, | ||
| "extraArrayOfStrings": []any{ | ||
| "stringValue", | ||
| "stringValue", | ||
| }, | ||
| "extraArrayOfNumbers": []any{ | ||
| int64(2217), | ||
| int64(2217), | ||
| }, | ||
| "extraArrayOfMixedTypes": []any{ | ||
| "stringValue", | ||
| int64(2217), | ||
| true, | ||
| nil, | ||
| }, | ||
| "extraArrayWithNesting": []any{ | ||
| "stringValue", | ||
| []any{ | ||
| int64(2217), | ||
| []any{ | ||
| true, nil, | ||
| }, | ||
| }, | ||
| }, | ||
| "extraObject": map[string]any{ | ||
| "stringValue": "stringValue", | ||
| "numberValue": int64(2217), | ||
| "booleanValue": true, | ||
| "nullValue": nil, | ||
| }, | ||
| "extraObjectWithNesting": map[string]any{ | ||
| "stringValue": "stringValue", | ||
| "numberValue": int64(2217), | ||
| "booleanValue": true, | ||
| "nullValue": nil, | ||
| "arrayValue": []any{ | ||
| "stringValue", | ||
| int64(2217), | ||
| }, | ||
| "objectValue": map[string]any{ | ||
| "stringValue": "stringValue", | ||
| "numberValue": int64(2217), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| expectError: false, | ||
| }, | ||
| { | ||
| desc: "extension text", | ||
| slice: []event{ | ||
|
|
@@ -820,6 +924,13 @@ func TestCreateLogs(t *testing.T) { | |
| require.Equal(t, expected.severityText, logRecord.SeverityText()) | ||
| require.Equal(t, expected.severityNumber, logRecord.SeverityNumber()) | ||
| require.Equal(t, expected.body, logRecord.Body().Str()) | ||
|
|
||
| // Check extra attributes | ||
| for key, value := range expected.attributes { | ||
| attr, ok := logRecord.Attributes().Get(key) | ||
| require.True(t, ok, "expected attribute %s not found", key) | ||
| require.Equal(t, value, attr.AsRaw(), "expected attribute %s to have value %v, got %v", key, value, attr.AsRaw()) | ||
| } | ||
| } | ||
| }) | ||
| } | ||
|
|
||
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.
You really should include
"type"here as well. Look at:opentelemetry-lambda/collector/receiver/telemetryapireceiver/receiver.go
Line 430 in e0f470f
Every log record gets an attribute with key "type" and a value like "function" or "platform.start", = the event type from the Telemetry API.
Now what if we have a log line originating from a user's lambda function code:
{ "message": "buy order", "type": "checkout", "orderId": "123" }So now in your newly added code below (in the default branch) you will simply overwrite the
typeattribute. And its value will now be"checkout"instead of"function"...Another, maybe better, option would be to prefix the additional fields that are in a user's logs so we can keep both.
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.
That was an oversight, good catch.
In the first place I was thinking about adding prefixes to the fields but after some thought I abandoned this option. Developers should be familiar with limitations and reserved fields and should handle that themselves on business logic level.
But maybe it's worth to update documentation and point out all the reserved internal fields with info that they will be prioritized over users metadata. WDYT?