feat(log): added log statements to gend autocfg #1053

diegomarquezp · 2022-10-03T22:11:39Z

Added log statements and setup in autoconfig class. Some common methods were put in Util in case they have to be reused

zhumin8

Looks good in general. Got a couple of minor comments below.
Other than that, you might want to rebase to take in #1054 I merged in earlier. Sorry about the timing.

src/test/java/com/google/api/generator/spring/SpringAutoConfigClassComposerTest.java

zhumin8 · 2022-10-04T18:28:34Z

src/test/java/com/google/api/generator/spring/SpringAutoConfigClassComposerTest.java

          + "    }\n"
          + "    if (this.clientProperties.getExecutorThreadCount() != null) {\n"
          + "      ExecutorProvider executorProvider =\n"
          + "          EchoSettings.defaultExecutorProviderBuilder()\n"
          + "              .setExecutorThreadCount(this.clientProperties.getExecutorThreadCount())\n"
          + "              .build();\n"
          + "      clientSettingsBuilder.setBackgroundExecutorProvider(executorProvider);\n"
+          + "      LOGGER.info(\"Background executor set to \" + executorProvider.getClass().getName());\n"


It's probably more useful to expose this.clientProperties.getExecutorThreadCount() instead in the log here.

zhumin8 · 2022-10-04T18:37:21Z

src/test/java/com/google/api/generator/spring/SpringAutoConfigClassComposerTest.java

          + "    }\n"
          + "    if (this.clientProperties.getExecutorThreadCount() != null) {\n"
          + "      ExecutorProvider executorProvider =\n"
          + "          EchoSettings.defaultExecutorProviderBuilder()\n"
          + "              .setExecutorThreadCount(this.clientProperties.getExecutorThreadCount())\n"
          + "              .build();\n"
          + "      clientSettingsBuilder.setBackgroundExecutorProvider(executorProvider);\n"
+          + "      LOGGER.info(\"Background executor set to \" + executorProvider.getClass().getName());\n"
          + "    }\n"
          + "    if (this.clientProperties.getUseRest()) {\n"
          + "      clientSettingsBuilder.setTransportChannelProvider(\n"


I only had the quota-project-id and thread-count in mind for the loggings initially, which you already implemented.
But it might be beneficial to also add logger statement inside each of the if statements? So user can have more dubug info about properties that are set on top of defaults. What do you think?

zhumin8 · 2022-10-04T18:46:02Z

src/main/java/com/google/api/generator/spring/composer/Utils.java

@@ -136,6 +144,56 @@ public static List<? extends AstNode> processRetrySettings(
    return resultList;
  }

+  public static Statement getLoggerDeclarationExpr(String className, Map<String, TypeNode> types) {


hmm, Perhaps add it to a separate LoggerUtils.java and put both under a utils folder? I feel everything in this one class starts to feel cluttered. I'll separate out unrelated methods in current Utils.java later in a separate pr.

src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java

emmileaf

LGTM (and apologies again for the conflicts introduced from earlier PRs!)

Added log statements and setup in autoconfig class. Some common methods were put in Util in case they have to be reused

- added rest of logger statements (1 per if-statement) - used getExecutorThreadCount() to log number of threads - moved logging utils to its own class

sonarqubecloud · 2022-10-22T22:06:39Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
14 Code Smells

95.7% Coverage
0.0% Duplication

* feat(log): added log statements to gend autocfg Added log statements and setup in autoconfig class. Some common methods were put in Util in case they have to be reused * fix(log): fixes from PR comments - added rest of logger statements (1 per if-statement) - used getExecutorThreadCount() to log number of threads - moved logging utils to its own class * fix(format): ran google_java_format * fix(test): updated goldens * fix: add license header to LoggerUtil

…tatic types instead of vapor references (#1046) Adding Spring related dependencies to build files and use static types instead of vapor references. Benefit of this change: - easier to test - less code change needed for future changes (e.g. package change of classes). One caveat is that because `gapic-generator-java` is brought into `googleapis` as `http_archive` code addition to `googleapis` is also needed to include these libraries. Add below code snippet to [WORKSPACE](https://github.com/googleapis/googleapis/blob/38e8b447d173909d3b2fe8fdc3e6cbb3c85442fd/WORKSPACE#L239-L245): ``` SPRING_MAVEN_ARTIFACTS = [ "org.springframework.boot:spring-boot-starter:2.7.4", "com.google.cloud:spring-cloud-gcp-core:3.3.0", ] maven_install( artifacts = PROTOBUF_MAVEN_ARTIFACTS + SPRING_MAVEN_ARTIFACTS, generate_compat_repositories = True, repositories = [ "https://repo.maven.apache.org/maven2/", ], ) ``` ------------ Updates: - Updated pr with #1065, - Moved annotation classed added in #1045 to static types. - Fixed conflicts from merged changes in #1093 - Moved logging classes added in #1053 to static types.

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.api:api-common](https://togithub.com/googleapis/api-common-java) | `2.2.2` -> `2.3.1` | [![age](https://badges.renovateapi.com/packages/maven/com.google.api:api-common/2.3.1/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.api:api-common/2.3.1/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.api:api-common/2.3.1/compatibility-slim/2.2.2)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.api:api-common/2.3.1/confidence-slim/2.2.2)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>googleapis/api-common-java</summary> ### [`v2.3.1`](https://togithub.com/googleapis/api-common-java/blob/HEAD/CHANGELOG.md#231-httpsgithubcomgoogleapisapi-common-javacomparev230v231-2022-12-06) [Compare Source](https://togithub.com/googleapis/api-common-java/compare/v2.2.2...v2.3.1) ##### Bug Fixes - Build with JDK11 and target JDK8 ([1862611](https://togithub.com/googleapis/api-common-java/commit/1862611d8b5cff69288380fe567e6ce033148c55)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-core).

diegomarquezp requested review from a team as code owners October 3, 2022 22:11

diegomarquezp requested review from zhumin8, blakeli0, elefeint and emmileaf October 3, 2022 22:13

diegomarquezp force-pushed the autoconfig-gen-draft2-logger branch from e1e4b56 to fa7963e Compare October 3, 2022 22:21

zhumin8 requested changes Oct 4, 2022

View reviewed changes

emmileaf mentioned this pull request Oct 4, 2022

test(Spring CodeGen): Set up golden file tests for spring class composers #1052

Merged

emmileaf reviewed Oct 4, 2022

View reviewed changes

src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java Outdated Show resolved Hide resolved

emmileaf added the spring pr that's related to spring code gen, intend to merge into autoconfig-gen-draft2 branch. label Oct 4, 2022

diegomarquezp force-pushed the autoconfig-gen-draft2-logger branch from 3579b7c to 854f34a Compare October 5, 2022 17:54

diegomarquezp force-pushed the autoconfig-gen-draft2-logger branch from 8832d0c to 28269bd Compare October 12, 2022 20:22

emmileaf approved these changes Oct 13, 2022

View reviewed changes

zhumin8 approved these changes Oct 17, 2022

View reviewed changes

diegomarquezp added 5 commits October 22, 2022 22:03

feat(log): added log statements to gend autocfg

16fe2d7

Added log statements and setup in autoconfig class. Some common methods were put in Util in case they have to be reused

fix(log): fixes from PR comments

a82b49d

- added rest of logger statements (1 per if-statement) - used getExecutorThreadCount() to log number of threads - moved logging utils to its own class

fix(format): ran google_java_format

c136db2

fix(test): updated goldens

9f20ae9

fix: add license header to LoggerUtil

5041b71

diegomarquezp force-pushed the autoconfig-gen-draft2-logger branch from 0a0c92b to 5041b71 Compare October 22, 2022 22:03

diegomarquezp merged commit 1818d0b into autoconfig-gen-draft2 Oct 22, 2022

diegomarquezp deleted the autoconfig-gen-draft2-logger branch October 22, 2022 22:04

zhumin8 mentioned this pull request Nov 17, 2022

fix(Spring CodeGen): Add Spring dependencies to build files and use static types instead of vapor references #1046

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(log): added log statements to gend autocfg #1053

feat(log): added log statements to gend autocfg #1053

diegomarquezp commented Oct 3, 2022

zhumin8 left a comment •

edited

Loading

zhumin8 Oct 4, 2022

zhumin8 Oct 4, 2022

zhumin8 Oct 4, 2022

emmileaf left a comment

sonarqubecloud bot commented Oct 22, 2022

feat(log): added log statements to gend autocfg #1053

feat(log): added log statements to gend autocfg #1053

Conversation

diegomarquezp commented Oct 3, 2022

zhumin8 left a comment • edited Loading

Choose a reason for hiding this comment

zhumin8 Oct 4, 2022

Choose a reason for hiding this comment

zhumin8 Oct 4, 2022

Choose a reason for hiding this comment

zhumin8 Oct 4, 2022

Choose a reason for hiding this comment

emmileaf left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Oct 22, 2022

zhumin8 left a comment •

edited

Loading