Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(logging): added marshalling methods for proto fields in structuredLogEntry #8979

Merged
merged 4 commits into from
Nov 13, 2023
Merged

fix(logging): added marshalling methods for proto fields in structuredLogEntry #8979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bdea556
fix(logging): added marshalling methods for proto fields in
gkevinzheng Nov 1, 2023
5dbaf2a
Renamed structuredLogEntryHttpRequest due to linter error.
gkevinzheng Nov 6, 2023
d35c800
Merge branch 'main' into logging-json-marshal-fix
gkevinzheng Nov 7, 2023
44d2700
Merge branch 'main' into logging-json-marshal-fix
gkevinzheng Nov 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
100 changes: 53 additions & 47 deletions logging/logging.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import (
logtypepb "google.golang.org/genproto/googleapis/logging/type"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/types/known/anypb"
"google.golang.org/protobuf/types/known/timestamppb"
)
Expand Down Expand Up @@ -967,65 +968,70 @@ func toLogEntryInternalImpl(e Entry, l *Logger, parent string, skipLevels int) (
// entry represents the fields of a logging.Entry that can be parsed by Logging agent.
// See the mappings at https://cloud.google.com/logging/docs/structured-logging#special-payload-fields
type structuredLogEntry struct {
// JsonMessage map[string]interface{} `json:"message,omitempty"`
// TextMessage string `json:"message,omitempty"`
Message json.RawMessage `json:"message"`
Severity string `json:"severity,omitempty"`
HTTPRequest *logtypepb.HttpRequest `json:"httpRequest,omitempty"`
Timestamp string `json:"timestamp,omitempty"`
Labels map[string]string `json:"logging.googleapis.com/labels,omitempty"`
InsertID string `json:"logging.googleapis.com/insertId,omitempty"`
Operation *logpb.LogEntryOperation `json:"logging.googleapis.com/operation,omitempty"`
SourceLocation *logpb.LogEntrySourceLocation `json:"logging.googleapis.com/sourceLocation,omitempty"`
SpanID string `json:"logging.googleapis.com/spanId,omitempty"`
Trace string `json:"logging.googleapis.com/trace,omitempty"`
TraceSampled bool `json:"logging.googleapis.com/trace_sampled,omitempty"`
Message json.RawMessage `json:"message"`
Severity string `json:"severity,omitempty"`
HTTPRequest *structuredLogEntryHTTPRequest `json:"httpRequest,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since datatype of HTTPRequest is being changed, this can be a breaking change if the user is able to access this field. I see that structuredLogEntry type itself is unexported. So, it is highly unlikely but please recheck the rest of the logging code to ensure the same.

Timestamp string `json:"timestamp,omitempty"`
Labels map[string]string `json:"logging.googleapis.com/labels,omitempty"`
InsertID string `json:"logging.googleapis.com/insertId,omitempty"`
Operation *structuredLogEntryOperation `json:"logging.googleapis.com/operation,omitempty"`
SourceLocation *structuredLogEntrySourceLocation `json:"logging.googleapis.com/sourceLocation,omitempty"`
SpanID string `json:"logging.googleapis.com/spanId,omitempty"`
Trace string `json:"logging.googleapis.com/trace,omitempty"`
TraceSampled bool `json:"logging.googleapis.com/trace_sampled,omitempty"`
}

func convertSnakeToMixedCase(snakeStr string) string {
words := strings.Split(snakeStr, "_")
mixedStr := words[0]
for _, word := range words[1:] {
mixedStr += strings.Title(word)
}
return mixedStr
// structuredLogEntryHTTPRequest wraps the HTTPRequest proto field in structuredLogEntry for easier JSON marshalling.
type structuredLogEntryHTTPRequest struct {
request *logtypepb.HttpRequest
}

func (s structuredLogEntry) MarshalJSON() ([]byte, error) {
// extract structuredLogEntry into json map
type Alias structuredLogEntry
var mapData map[string]interface{}
data, err := json.Marshal(Alias(s))
if err == nil {
err = json.Unmarshal(data, &mapData)
}
if err == nil {
// ensure all inner dicts use mixed case instead of snake case
innerDicts := [3]string{"httpRequest", "logging.googleapis.com/operation", "logging.googleapis.com/sourceLocation"}
for _, field := range innerDicts {
if fieldData, ok := mapData[field]; ok {
formattedFieldData := make(map[string]interface{})
for k, v := range fieldData.(map[string]interface{}) {
formattedFieldData[convertSnakeToMixedCase(k)] = v
}
mapData[field] = formattedFieldData
}
}
// serialize json map into raw bytes
return json.Marshal(mapData)
}
return data, err
func (s structuredLogEntryHTTPRequest) MarshalJSON() ([]byte, error) {
return protojson.Marshal(s.request)
}

// structuredLogEntryOperation wraps the Operation proto field in structuredLogEntry for easier JSON marshalling.
type structuredLogEntryOperation struct {
operation *logpb.LogEntryOperation
}

func (s structuredLogEntryOperation) MarshalJSON() ([]byte, error) {
return protojson.Marshal(s.operation)
}

// structuredLogEntrySourceLocation wraps the SourceLocation proto field in structuredLogEntry for easier JSON marshalling.
type structuredLogEntrySourceLocation struct {
sourceLocation *logpb.LogEntrySourceLocation
}

func (s structuredLogEntrySourceLocation) MarshalJSON() ([]byte, error) {
return protojson.Marshal(s.sourceLocation)
}

func serializeEntryToWriter(entry *logpb.LogEntry, w io.Writer) error {
var httpRequest *structuredLogEntryHTTPRequest
if entry.HttpRequest != nil {
httpRequest = &structuredLogEntryHTTPRequest{entry.HttpRequest}
}

var operation *structuredLogEntryOperation
if entry.Operation != nil {
operation = &structuredLogEntryOperation{entry.Operation}
}

var sourceLocation *structuredLogEntrySourceLocation
if entry.SourceLocation != nil {
sourceLocation = &structuredLogEntrySourceLocation{entry.SourceLocation}
}

jsonifiedEntry := structuredLogEntry{
Severity: entry.Severity.String(),
HTTPRequest: entry.HttpRequest,
HTTPRequest: httpRequest,
Timestamp: entry.Timestamp.String(),
Labels: entry.Labels,
InsertID: entry.InsertId,
Operation: entry.Operation,
SourceLocation: entry.SourceLocation,
Operation: operation,
SourceLocation: sourceLocation,
SpanID: entry.SpanId,
Trace: entry.Trace,
TraceSampled: entry.TraceSampled,
Expand Down
20 changes: 17 additions & 3 deletions logging/logging_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1438,7 +1438,7 @@ func TestRedirectOutputFormats(t *testing.T) {
},
want: `{"httpRequest":{"requestMethod":"POST","requestUrl":"https://example.com/test"},"logging.googleapis.com/insertId":"0000AAA01",` +
`"logging.googleapis.com/labels":{"key1":"value1","key2":"value2"},"logging.googleapis.com/operation":{"id":"0123456789","producer":"test"},` +
`"logging.googleapis.com/sourceLocation":{"file":"acme.go","function":"main","line":100},"logging.googleapis.com/spanId":"000000000001",` +
`"logging.googleapis.com/sourceLocation":{"file":"acme.go","function":"main","line":"100"},"logging.googleapis.com/spanId":"000000000001",` +
`"logging.googleapis.com/trace":"projects/P/ABCD12345678AB12345678","logging.googleapis.com/trace_sampled":true,` +
`"message":"this is text payload","severity":"DEBUG","timestamp":"seconds:1000"}`,
},
Expand Down Expand Up @@ -1474,7 +1474,7 @@ func TestRedirectOutputFormats(t *testing.T) {
},
want: `{"httpRequest":{"requestMethod":"POST","requestUrl":"https://example.com/test"},"logging.googleapis.com/insertId":"0000AAA01",` +
`"logging.googleapis.com/labels":{"key1":"value1","key2":"value2"},"logging.googleapis.com/operation":{"id":"0123456789","producer":"test"},` +
`"logging.googleapis.com/sourceLocation":{"file":"acme.go","function":"main","line":100},"logging.googleapis.com/spanId":"000000000001",` +
`"logging.googleapis.com/sourceLocation":{"file":"acme.go","function":"main","line":"100"},"logging.googleapis.com/spanId":"000000000001",` +
`"logging.googleapis.com/trace":"projects/P/ABCD12345678AB12345678","logging.googleapis.com/trace_sampled":true,` +
`"message":{"Latency":321,"Message":"message part of the payload"},"severity":"DEBUG","timestamp":"seconds:1000"}`,
},
Expand Down Expand Up @@ -1506,7 +1506,21 @@ func TestRedirectOutputFormats(t *testing.T) {
t.Errorf("Expected error: %+v, want: %v\n", tc.in, tc.wantError)
}
got := strings.TrimSpace(buffer.String())
if got != tc.want {

// Compare structure equivalence of the outputs, not string equivalence, as order doesn't matter.
var gotJson, wantJson interface{}

err = json.Unmarshal([]byte(got), &gotJson)
if err != nil {
t.Errorf("Error when serializing JSON output: %v", err)
}

err = json.Unmarshal([]byte(tc.want), &wantJson)
if err != nil {
t.Fatalf("Error unmarshalling JSON input for want: %v", err)
}

if !reflect.DeepEqual(gotJson, wantJson) {
t.Errorf("TestRedirectOutputFormats: %+v: got %v, want %v", tc.in, got, tc.want)
}
}
Expand Down