|
| 1 | +# Style Guide |
| 2 | + |
| 3 | +This project follows the |
| 4 | +[Google Java Style Guide](https://google.github.io/styleguide/javaguide.html). |
| 5 | + |
| 6 | +## Code Formatting |
| 7 | + |
| 8 | +### Auto-formatting |
| 9 | + |
| 10 | +The build will fail if source code is not formatted according to Google Java Style. |
| 11 | + |
| 12 | +Run the following command to reformat all files: |
| 13 | + |
| 14 | +```bash |
| 15 | +./gradlew spotlessApply |
| 16 | +``` |
| 17 | + |
| 18 | +For IntelliJ users, an `.editorconfig` file is provided that IntelliJ will automatically use to |
| 19 | +adjust code formatting settings. However, it does not support all required rules, so you may still |
| 20 | +need to run `./gradlew spotlessApply` periodically. |
| 21 | + |
| 22 | +### Static imports |
| 23 | + |
| 24 | +Consider statically importing the following commonly used methods and constants: |
| 25 | + |
| 26 | +- **Test methods** |
| 27 | + - `io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions` (assertThat, satisfies, etc.) |
| 28 | + - `org.assertj.core.api.Assertions.*` (assertThat, assertThatThrownBy, entry, etc.) |
| 29 | + - `org.mockito.Mockito.*` (when, mock, verify, times, etc.) |
| 30 | + - `org.mockito.ArgumentMatchers.*` (any, eq, anyLong, etc.) |
| 31 | +- **Utility methods** |
| 32 | + - `io.opentelemetry.api.common.AttributeKey.*` (stringKey, longKey, etc.) |
| 33 | + - `java.util.Arrays.*` (asList, etc.) |
| 34 | + - `java.util.Collections.*` (singletonList, emptyList, emptyMap, emptySet, etc.) |
| 35 | + - `java.util.stream.Collectors.*` (toList, toMap, toSet, etc.) |
| 36 | + - `java.util.Objects` (requireNonNull, etc.) |
| 37 | + - `java.util.logging.Level.*` (FINE, INFO, WARNING, etc.) |
| 38 | + - `java.nio.charset.StandardCharsets.*` (UTF_8, etc.) |
| 39 | +- **Time unit constants** |
| 40 | + - `java.util.concurrent.TimeUnit.*` (SECONDS, etc.) |
| 41 | +- **OpenTelemetry semantic convention constants** |
| 42 | + - All constants under `io.opentelemetry.semconv.**` |
| 43 | + |
| 44 | +### Class organization |
| 45 | + |
| 46 | +Prefer this order: |
| 47 | + |
| 48 | +- Static fields (final before non-final) |
| 49 | +- Instance fields (final before non-final) |
| 50 | +- Constructors |
| 51 | +- Methods |
| 52 | +- Nested classes |
| 53 | + |
| 54 | +**Method ordering**: Place calling methods above the methods they call. For example, place private |
| 55 | +methods below the non-private methods that use them. |
| 56 | + |
| 57 | +**Static utility classes**: Place the private constructor (used to prevent instantiation) after all |
| 58 | +methods. |
| 59 | + |
| 60 | +## Java Language Conventions |
| 61 | + |
| 62 | +### Package conventions |
| 63 | + |
| 64 | +Classes in `.internal` packages are not considered public API and may change without notice. These |
| 65 | +packages contain implementation details that should not be used by external consumers. |
| 66 | + |
| 67 | +- Use `.internal` packages for implementation classes that need to be public within the module but |
| 68 | + should not be used externally |
| 69 | +- Try to avoid referencing `.internal` classes from other modules |
| 70 | + |
| 71 | +### `final` keyword usage |
| 72 | + |
| 73 | +Public API classes should be declared `final` where possible. |
| 74 | + |
| 75 | +Methods should only be declared `final` if they are in non-final public API classes. |
| 76 | + |
| 77 | +Fields should be declared `final` where possible. |
| 78 | + |
| 79 | +Method parameters and local variables should never be declared `final`. |
| 80 | + |
| 81 | +### `@Nullable` annotation usage |
| 82 | + |
| 83 | +**Note: This section is aspirational and may not reflect the current codebase.** |
| 84 | + |
| 85 | +Annotate all parameters and fields that can be `null` with `@Nullable` (specifically |
| 86 | +`javax.annotation.Nullable`, which is included by the `otel.java-conventions` Gradle plugin as a |
| 87 | +`compileOnly` dependency). |
| 88 | + |
| 89 | +`@NonNull` is unnecessary as it is the default. |
| 90 | + |
| 91 | +**Defensive programming**: Public APIs should still check for `null` parameters even if not |
| 92 | +annotated with `@Nullable`. Internal APIs do not need these checks. |
| 93 | + |
| 94 | +**Enforcement**: Use the `otel.errorprone-conventions` Gradle plugin in all modules: |
| 95 | + |
| 96 | +```kotlin |
| 97 | +plugins { |
| 98 | + id("otel.errorprone-conventions") |
| 99 | +} |
| 100 | +``` |
| 101 | + |
| 102 | +### `Optional` usage |
| 103 | + |
| 104 | +Following the reasoning from |
| 105 | +[Writing a Java library with better experience (slide 12)](https://speakerdeck.com/trustin/writing-a-java-library-with-better-experience?slide=12), |
| 106 | +`java.util.Optional` usage is kept to a minimum. |
| 107 | + |
| 108 | +**Guidelines**: |
| 109 | + |
| 110 | +- `Optional` shouldn't appear in public API signatures |
| 111 | +- Avoid `Optional` on the hot path (instrumentation code), unless the instrumented library uses it |
| 112 | + |
| 113 | +## Tooling conventions |
| 114 | + |
| 115 | +### AssertJ |
| 116 | + |
| 117 | +Prefer AssertJ assertions over JUnit assertions (assertEquals, assertTrue, etc.) for better error |
| 118 | +messages. |
| 119 | + |
| 120 | +### AutoService |
| 121 | + |
| 122 | +Use the `@AutoService` annotation when implementing SPI interfaces. This automatically generates the |
| 123 | +necessary `META-INF/services/` files at compile time, eliminating the need to manually create and |
| 124 | +maintain service registration files. |
| 125 | + |
| 126 | +```java |
| 127 | +@AutoService(AutoConfigurationCustomizerProvider.class) |
| 128 | +public class MyCustomizerProvider implements AutoConfigurationCustomizerProvider { |
| 129 | + // implementation |
| 130 | +} |
| 131 | +``` |
| 132 | + |
| 133 | +### Gradle |
| 134 | + |
| 135 | +- Use Kotlin instead of Groovy for build scripts |
| 136 | +- Plugin versions should be specified in `settings.gradle.kts`, not in individual modules |
| 137 | +- All modules should use `plugins { id("otel.java-conventions") }` |
| 138 | +- Set module names with `otelJava.moduleName.set("io.opentelemetry.contrib.mymodule")` |
| 139 | + |
| 140 | +## Configuration |
| 141 | + |
| 142 | +- Use `otel.` prefix for all configuration property keys |
| 143 | +- Read config via the `ConfigProperties` interface |
| 144 | +- Provide sensible defaults and document all options |
| 145 | +- Validate configuration early with clear error messages |
| 146 | + |
| 147 | +## Performance |
| 148 | + |
| 149 | +Avoid allocations on the hot path (instrumentation code) whenever possible. This includes `Iterator` |
| 150 | +allocations from collections; note that `for (SomeType t : plainJavaArray)` does not allocate an |
| 151 | +iterator object. |
| 152 | + |
| 153 | +Non-allocating Stream API usage on the hot path is acceptable but may not fit the surrounding code |
| 154 | +style; this is a judgment call. Some Stream APIs make efficient allocation difficult (e.g., |
| 155 | +`collect` with pre-sized sink data structures involves convoluted `Supplier` code, or lambdas passed |
| 156 | +to `forEach` may be capturing/allocating lambdas). |
| 157 | + |
| 158 | +## Documentation |
| 159 | + |
| 160 | +### Component README files |
| 161 | + |
| 162 | +- Include a component owners section in each module's README |
| 163 | +- Document configuration options with examples |
| 164 | + |
| 165 | +### Deprecation and breaking changes |
| 166 | + |
| 167 | +Breaking changes are allowed in unstable modules (published with `-alpha` version suffix). |
| 168 | + |
| 169 | +1. Mark APIs with `@Deprecated` and a removal timeline (there must be at least one release with the |
| 170 | + API marked as deprecated before removing it) |
| 171 | +2. Document the replacement in Javadoc with `@deprecated` tag |
| 172 | +3. Note the migration path for breaking changes under a "Migration notes" section of CHANGELOG.md |
| 173 | + (create this section at the top of the Unreleased section if not already present) |
0 commit comments