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.

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

feat: add custom logging provider to support structured logging for SLF4J 1.x + Logback 1.2.x #3693

Open
wants to merge 66 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
60ec4e6
feat: add sdk logging helpers
JoeWang1127 Mar 12, 2025
88097e8
refactor
JoeWang1127 Mar 12, 2025
bd30ffa
add test deps
JoeWang1127 Mar 12, 2025
b32e11b
add test
JoeWang1127 Mar 12, 2025
4181927
add formatter
JoeWang1127 Mar 12, 2025
98e4130
Merge branch 'main' into feat/sdk-logging-helper
JoeWang1127 Mar 12, 2025
209768b
format
JoeWang1127 Mar 12, 2025
b0310a4
change version def
JoeWang1127 Mar 12, 2025
64acdfb
update test
JoeWang1127 Mar 12, 2025
82ff79d
add profile
JoeWang1127 Mar 12, 2025
e0f8981
skip normal release
JoeWang1127 Mar 12, 2025
204fb8f
add test
JoeWang1127 Mar 12, 2025
925f04c
update test
JoeWang1127 Mar 12, 2025
469c348
update test
JoeWang1127 Mar 12, 2025
25a6cb3
format
JoeWang1127 Mar 12, 2025
d1698a1
add test
JoeWang1127 Mar 13, 2025
b4effd9
remove slf4j deps
JoeWang1127 Mar 13, 2025
fb42331
add parent
JoeWang1127 Mar 13, 2025
d0712cc
update pom
JoeWang1127 Mar 13, 2025
e8eda21
add skip release in parent pom
JoeWang1127 Mar 13, 2025
b6819d8
Merge branch 'main' into feat/sdk-logging-helper
JoeWang1127 Mar 13, 2025
f35a134
Merge branch 'main' into feat/sdk-logging-helper
JoeWang1127 Mar 14, 2025
25705f1
Merge branch 'main' into feat/sdk-logging-helper
JoeWang1127 Mar 14, 2025
0bc9c94
add a bom
JoeWang1127 Mar 14, 2025
710705b
add a empty class for test prep
JoeWang1127 Mar 14, 2025
d56cf81
Merge branch 'main' into feat/sdk-logging-helper
JoeWang1127 Mar 14, 2025
310e546
chore: generate libraries at Fri Mar 14 19:00:09 UTC 2025
cloud-java-bot Mar 14, 2025
8bfd02a
remove optional
JoeWang1127 Mar 14, 2025
7c8e653
add showcase tests
JoeWang1127 Mar 14, 2025
99bf64b
chore: generate libraries at Fri Mar 14 23:07:02 UTC 2025
cloud-java-bot Mar 14, 2025
dc73a51
remove child version
JoeWang1127 Mar 14, 2025
c029d4f
update showcase tests
JoeWang1127 Mar 15, 2025
d3a8988
exclude compile appender in 1.x
JoeWang1127 Mar 15, 2025
9fd9d56
exclude compile appender in 1.x
JoeWang1127 Mar 15, 2025
68c7a3c
exclude compile appender in 1.x
JoeWang1127 Mar 15, 2025
9e69796
add unit tests
JoeWang1127 Mar 15, 2025
4b88289
add early return
JoeWang1127 Mar 15, 2025
4de3bfc
refactor
JoeWang1127 Mar 15, 2025
ed28cd4
update tests
JoeWang1127 Mar 15, 2025
7529de0
rename
JoeWang1127 Mar 15, 2025
f295db8
chore: generate libraries at Sat Mar 15 15:30:16 UTC 2025
cloud-java-bot Mar 15, 2025
f54db56
add showcase test
JoeWang1127 Mar 15, 2025
e717447
add showcase test
JoeWang1127 Mar 15, 2025
8237028
refactor test
JoeWang1127 Mar 15, 2025
4aef523
format
JoeWang1127 Mar 15, 2025
f5efa2a
skip release
JoeWang1127 Mar 15, 2025
c26da4b
format
JoeWang1127 Mar 15, 2025
afb6ae6
merge main branch
JoeWang1127 Mar 18, 2025
12ea137
update root pom
JoeWang1127 Mar 18, 2025
cfc9af3
install logging module in showcase ci
JoeWang1127 Mar 18, 2025
9ea01fa
continue
JoeWang1127 Mar 18, 2025
89c469a
add unit tests
JoeWang1127 Mar 18, 2025
3179277
add included key
JoeWang1127 Mar 18, 2025
39f94ed
add unit tests
JoeWang1127 Mar 18, 2025
1c6f064
Merge branch 'main' into feat/sdk-logging-helper
JoeWang1127 Mar 18, 2025
b8db96f
format
JoeWang1127 Mar 18, 2025
6ed523e
add unit tests
JoeWang1127 Mar 18, 2025
6eb0f8e
change group id in showcase
JoeWang1127 Mar 18, 2025
2cc02e5
add key replacement
JoeWang1127 Mar 18, 2025
05fcd82
run test java 8
JoeWang1127 Mar 18, 2025
179ecc8
final
JoeWang1127 Mar 18, 2025
47b4221
Merge branch 'main' into feat/sdk-logging-helper
JoeWang1127 Mar 19, 2025
eea89ae
change package name
JoeWang1127 Mar 19, 2025
af93306
refactor
JoeWang1127 Mar 19, 2025
5898bd1
add comment
JoeWang1127 Mar 19, 2025
2c7ff17
Merge branch 'main' into feat/sdk-logging-helper
JoeWang1127 Mar 19, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,10 @@ jobs:
- name: Install Maven modules
run: |
mvn install -B -ntp -DskipTests -Dclirr.skip -Dcheckstyle.skip
- name: Install logging module # this module is not part of root project.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we want to install logging module in the main CI?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

run: |
mvn install -B -ntp -DskipTests -Dclirr.skip -Dcheckstyle.skip
working-directory: java-sdk-logging
- name: Java Linter
working-directory: java-showcase
run: |
Expand Down
23 changes: 19 additions & 4 deletions .github/workflows/java_sdk_logging.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,28 @@ jobs:
with:
java-version: 8
distribution: temurin
- run: echo "JAVA8_HOME=${JAVA_HOME}" >> $GITHUB_ENV
- uses: actions/setup-java@v4
with:
java-version: 17
distribution: temurin
- name: Install parent module
run: |
mvn install -B -ntp -pl gapic-generator-java-pom-parent
- name: Unit Tests
- name: Install logging module
run: |
mvn install -B -ntp -Dcheckstyle.skip -Dfmt.skip
working-directory: java-sdk-logging
- name: Unit Tests in Java 8
run: |
mvn test -B -ntp -Dcheckstyle.skip -Dfmt.skip
set -x
export JAVA_HOME=$JAVA_HOME
export PATH=${JAVA_HOME}/bin:$PATH
mvn verify -B -ntp \
-Dcheckstyle.skip \
-Dfmt.skip \
-Djvm="${JAVA8_HOME}/bin/java"
working-directory: java-sdk-logging
module-lint:
runs-on: ubuntu-latest
steps:
Expand All @@ -59,7 +74,7 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
with:
java-version: 11
java-version: 17
distribution: temurin
- name: Install parent module
run: |
Expand All @@ -74,7 +89,7 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
with:
java-version: 11
java-version: 17
distribution: temurin
- name: Install parent module
run: |
Expand Down
2 changes: 2 additions & 0 deletions java-sdk-logging/logback-extension/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<logback.version>1.2.13</logback.version>
<!-- do not update this version as this is the last version support
SLF4J 1.x and logback 1.2.x -->
<logstash.encoder.version>7.3</logstash.encoder.version>
</properties>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/*
* Copyright 2025 Google LLC
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
* met:
*
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following disclaimer
* in the documentation and/or other materials provided with the
* distribution.
* * Neither the name of Google LLC nor the names of its
* contributors may be used to endorse or promote products derived from
* this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

package com.google.cloud.sdk.logging;

import ch.qos.logback.classic.spi.ILoggingEvent;
import com.fasterxml.jackson.core.JsonGenerator;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is extending from MdcJsonProvider required? The writeTo interface exposes a jackson class JsonGenerator on the surface, and jackson is in general not recommended because of endless security issues.

Copy link
Collaborator Author

@JoeWang1127 JoeWang1127 Mar 19, 2025

Choose a reason for hiding this comment

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

The writeTo method is part of JsonProvider, which is implemented by MdcJsonProvider.

The JsonProvider is used in LogstashEncoder.

I don't think we can change the interface but we can extract jackson library and update to the lastest, what do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there an interface that does not expose jackson on the surface? if not, I think we may need to re-evaluate the feasibility of it. Since adding a new dependency, especially jackson, is a big commitment.

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.io.IOException;
import java.util.Map;
import net.logstash.logback.composite.loggingevent.MdcJsonProvider;

public class SDKLoggingMdcJsonProvider extends MdcJsonProvider {

private final ObjectMapper objectMapper = new ObjectMapper();

@Override
public void writeTo(JsonGenerator generator, ILoggingEvent event) throws IOException {
Map<String, String> mdcProperties = event.getMDCPropertyMap();
if (mdcProperties == null || mdcProperties.isEmpty()) {
return;
}

boolean hasWrittenStart = false;
for (Map.Entry<String, String> entry : mdcProperties.entrySet()) {
String fieldName = entry.getKey();
String entryValueString = entry.getValue();
// an entry will be skipped if one of the scenario happens:
// 1. key or value is null
if (fieldName == null || entryValueString == null) {
continue;
}
// 2. includeMdcKeyNames is not empty and the key is not in the list
if (!getIncludeMdcKeyNames().isEmpty() && !getIncludeMdcKeyNames().contains(fieldName)) {
continue;
}
// 3. excludeMdcKeyNames is not empty and the key is in the list
if (!getExcludeMdcKeyNames().isEmpty() && getExcludeMdcKeyNames().contains(fieldName)) {
continue;
}

fieldName = getMdcKeyFieldNames().getOrDefault(entry.getKey(), fieldName);
if (!hasWrittenStart && getFieldName() != null) {
generator.writeObjectFieldStart(getFieldName());
hasWrittenStart = true;
}
generator.writeFieldName(fieldName);

try {
generator.writeTree(convertToTreeNode(entryValueString));
} catch (JsonProcessingException e) {
// in case of conversion exception, just use String
generator.writeObject(entryValueString);
}
}
if (hasWrittenStart) {
generator.writeEndObject();
}
}

private JsonNode convertToTreeNode(String jsonString) throws JsonProcessingException {
return objectMapper.readTree(jsonString);
}
}
Loading
Loading