-
Notifications
You must be signed in to change notification settings - Fork 169
Add copilot review instructions #2170
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
Merged
trask
merged 10 commits into
open-telemetry:main
from
trask:add-copilot-review-instructions
Aug 26, 2025
Merged
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
ee36ce4
Add copilot review instructions
trask a294423
visibility
trask 066eb13
Also add basic coding agent instructions
trask b71cc41
clarifications
trask 52463e1
update
trask 218f229
fix
trask ae9711b
more
trask 3631faa
refine
trask d49f7c2
up
trask 6e36706
Update .github/copilot-instructions.md
trask File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # Copilot Instructions for OpenTelemetry Java Contrib | ||
|
|
||
| This repository provides observability instrumentation for Java applications. | ||
|
|
||
| ## Code Review Priorities | ||
|
|
||
| ### Style Guide Compliance | ||
|
|
||
| **PRIORITY**: Verify that all code changes follow the [Style Guide](../docs/style-guide.md). Check: | ||
|
|
||
| - Code formatting (auto-formatting, static imports, class organization) | ||
| - Java language conventions (`final` usage, `@Nullable` annotations, `Optional` usage) | ||
| - Performance constraints (hot path allocations) | ||
| - Implementation patterns (SPI registration, configuration conventions) | ||
| - Gradle conventions (Kotlin DSL, plugin usage, module naming) | ||
| - Documentation standards (README files, deprecation processes) | ||
|
|
||
| ### Critical Areas | ||
|
|
||
| - **Public APIs**: Changes affect downstream users and require careful review | ||
| - **Performance**: Instrumentation must have minimal overhead | ||
| - **Thread Safety**: Ensure safe concurrent access patterns | ||
| - **Memory Management**: Prevent leaks and excessive allocations | ||
|
|
||
| ### Quality Standards | ||
|
|
||
| - Proper error handling with appropriate logging levels | ||
| - OpenTelemetry specification and semantic convention compliance | ||
| - Resource cleanup and lifecycle management | ||
| - Comprehensive unit tests for new functionality | ||
|
|
||
| ## Coding Agent Instructions | ||
|
|
||
| When implementing changes or new features: | ||
|
|
||
| 1. Follow all [Style Guide](../docs/style-guide.md) conventions and the Code Review Priorities above | ||
| 2. Run tests to ensure they still pass (use `./gradlew test` and `./gradlew integrationTest` as needed) | ||
| 3. **Always run `./gradlew spotlessApply`** after making code changes to ensure proper formatting | ||
trask marked this conversation as resolved.
Show resolved
Hide resolved
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,82 +1,65 @@ | ||
| # Contributing | ||
|
|
||
| Welcome to the OpenTelemetry Java Contrib Repository! | ||
| Welcome to the OpenTelemetry Java Contrib repository! | ||
|
|
||
| ## Introduction | ||
|
|
||
| This repository focuses on providing tools and utilities for Java-based observability, such as remote JMX metric gathering and reporting. We’re excited to have you here! Whether you’re fixing a bug, adding a feature, or suggesting an idea, your contributions are invaluable. | ||
| This repository provides observability libraries and utilities for Java applications that complement | ||
| the [OpenTelemetry Java SDK](https://github.com/open-telemetry/opentelemetry-java) and | ||
| [OpenTelemetry Java Instrumentation](https://github.com/open-telemetry/opentelemetry-java-instrumentation) | ||
| projects. | ||
|
|
||
| Before submitting new features or changes to current functionality, it is recommended to first | ||
| [open an issue](https://github.com/open-telemetry/opentelemetry-java-contrib/issues/new) | ||
| and discuss your ideas or propose the changes you wish to make. | ||
|
|
||
| Questions? Ask in the OpenTelemetry [java channel](https://cloud-native.slack.com/archives/C014L2KCTE3) | ||
| Before submitting new features or changes, please consider | ||
| [opening an issue](https://github.com/open-telemetry/opentelemetry-java-contrib/issues/new) first to | ||
| discuss your ideas. | ||
|
|
||
| Pull requests for bug fixes are always welcome! | ||
|
|
||
| ## Pre-requisites | ||
|
|
||
| To work with this repository, ensure you have: | ||
|
|
||
| ### Tools: | ||
|
|
||
| Java 17 or higher | ||
|
|
||
| ### Platform Notes: | ||
| ## Building and Testing | ||
|
|
||
| macOS/Linux: Ensure JAVA_HOME is set correctly. | ||
| While most modules target Java 8, building this project requires Java 17 or higher. | ||
|
|
||
| ## Workflow | ||
|
|
||
| 1. Fork the repository | ||
| 2. Clone locally | ||
| 3. Create a branch before working on an issue | ||
|
|
||
| ## Local Run/Build | ||
|
|
||
| In order to build and test this whole repository you need JDK 11+. | ||
|
|
||
| #### Snapshot builds | ||
|
|
||
| For developers testing code changes before a release is complete, there are | ||
| snapshot builds of the `main` branch. They are available from | ||
| the Sonatype snapshot repository at `https://central.sonatype.com/repository/maven-snapshots/` | ||
| ([browse](https://central.sonatype.com/service/rest/repository/browse/maven-snapshots/io/opentelemetry/contrib/)). | ||
|
|
||
| #### Building from source | ||
|
|
||
| Building using Java 11+: | ||
| To build the project: | ||
|
|
||
| ```bash | ||
| $ java -version | ||
| ./gradlew assemble | ||
| ``` | ||
|
|
||
| To run the tests: | ||
|
|
||
| ```bash | ||
| $ ./gradlew assemble | ||
| ./gradlew test | ||
| ``` | ||
|
|
||
| ## Testing | ||
| Some modules include integration tests that can be run with: | ||
|
|
||
| ```bash | ||
| $ ./gradlew test | ||
| ./gradlew integrationTest | ||
| ``` | ||
|
|
||
| ### Some modules have integration tests | ||
| ## Snapshot Builds | ||
|
|
||
| ``` | ||
| $ ./gradlew integrationTest | ||
| ``` | ||
| Snapshot builds of the `main` branch are available from the Sonatype snapshot repository at: | ||
| `https://central.sonatype.com/repository/maven-snapshots/` | ||
| ([browse](https://central.sonatype.com/service/rest/repository/browse/maven-snapshots/io/opentelemetry/contrib/)). | ||
|
|
||
| ## Style Guide | ||
|
|
||
| See [Style Guide](docs/style-guide.md). | ||
|
|
||
| Follow the Java Instrumentation [Style Guide](https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/contributing/style-guideline.md) from the opentelemetry-java-instrumentation repository. | ||
| ## Pull Request Guidelines | ||
|
|
||
| Failure? Check logs for errors or mismatched dependencies. | ||
| When submitting a pull request, please ensure that you: | ||
|
|
||
| ## Gradle conventions | ||
| - Clearly describe the change and its motivation | ||
| - Mention any breaking changes | ||
| - Include tests for new functionality | ||
| - Follow the [Style Guide](docs/style-guide.md) | ||
|
|
||
| - Use kotlin instead of groovy | ||
| - Plugin versions should be specified in `settings.gradle.kts`, not in individual modules | ||
| - All modules use `plugins { id("otel.java-conventions") }` | ||
| ## Getting Help | ||
|
|
||
| ## Further Help | ||
| If you need assistance or have questions: | ||
|
|
||
| Join [#otel-java](https://cloud-native.slack.com/archives/C014L2KCTE3) on OpenTelemetry Slack | ||
| - Post on the [#otel-java](https://cloud-native.slack.com/archives/C014L2KCTE3) Slack channel | ||
| - [Open an issue](https://github.com/open-telemetry/opentelemetry-java-contrib/issues/new/choose) in | ||
| this repository |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,180 @@ | ||
| # Style Guide | ||
|
|
||
| This project follows the | ||
| [Google Java Style Guide](https://google.github.io/styleguide/javaguide.html). | ||
|
|
||
| ## Code Formatting | ||
|
|
||
| ### Auto-formatting | ||
|
|
||
| The build will fail if source code is not formatted according to Google Java Style. | ||
|
|
||
| Run the following command to reformat all files: | ||
|
|
||
| ```bash | ||
| ./gradlew spotlessApply | ||
| ``` | ||
|
|
||
| For IntelliJ users, an `.editorconfig` file is provided that IntelliJ will automatically use to | ||
| adjust code formatting settings. However, it does not support all required rules, so you may still | ||
| need to run `./gradlew spotlessApply` periodically. | ||
|
|
||
| ### Static imports | ||
|
|
||
| Consider statically importing the following commonly used methods and constants: | ||
|
|
||
| - **Test methods** | ||
| - `io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.*` | ||
| - `org.assertj.core.api.Assertions.*` | ||
| - `org.mockito.Mockito.*` | ||
| - `org.mockito.ArgumentMatchers.*` | ||
| - **Utility methods** | ||
| - `io.opentelemetry.api.common.AttributeKey.*` | ||
| - `java.util.Arrays` - asList, stream | ||
| - `java.util.Collections` - singleton*, empty*, unmodifiable*, synchronized*, checked* | ||
| - `java.util.Objects` - requireNonNull | ||
| - `java.util.function.Function` - identity | ||
| - `java.util.stream.Collectors.*` | ||
| - **Utility constants** | ||
| - `java.util.Locale.*` | ||
| - `java.util.concurrent.TimeUnit.*` | ||
| - `java.util.logging.Level.*` | ||
| - `java.nio.charset.StandardCharsets.*` | ||
| - **OpenTelemetry semantic convention constants** | ||
| - All constants under `io.opentelemetry.semconv.**`, except for | ||
| `io.opentelemetry.semconv.SchemaUrls.*` constants. | ||
|
|
||
| ### Class organization | ||
|
|
||
| Prefer this order: | ||
|
|
||
| - Static fields (final before non-final) | ||
| - Instance fields (final before non-final) | ||
| - Constructors | ||
| - Methods | ||
| - Nested classes | ||
|
|
||
| **Method ordering**: Place calling methods above the methods they call. For example, place private | ||
| methods below the non-private methods that use them. | ||
|
|
||
| **Static utility classes**: Place the private constructor (used to prevent instantiation) after all | ||
| methods. | ||
|
|
||
| ## Java Language Conventions | ||
|
|
||
| ### Visibility modifiers | ||
|
|
||
| Follow the principle of minimal necessary visibility. Use the most restrictive access modifier that | ||
| still allows the code to function correctly. | ||
|
|
||
| ### Internal packages | ||
|
|
||
| Classes in `.internal` packages are not considered public API and may change without notice. These | ||
| packages contain implementation details that should not be used by external consumers. | ||
|
|
||
| - Use `.internal` packages for implementation classes that need to be public within the module but | ||
| should not be used externally | ||
| - Try to avoid referencing `.internal` classes from other modules | ||
|
|
||
| ### `final` keyword usage | ||
|
|
||
| Public non-internal classes should be declared `final` where possible. Internal and non-public | ||
| classes should not be declared `final`. | ||
|
|
||
| Methods should only be declared `final` if they are in public non-internal non-final classes. | ||
|
|
||
| Fields should be declared `final` where possible. | ||
|
|
||
| Method parameters and local variables should never be declared `final`. | ||
|
|
||
| ### `@Nullable` annotation usage | ||
|
|
||
| **Note: This section is aspirational and may not reflect the current codebase.** | ||
|
|
||
| Annotate all parameters and fields that can be `null` with `@Nullable` (specifically | ||
| `javax.annotation.Nullable`, which is included by the `otel.java-conventions` Gradle plugin as a | ||
| `compileOnly` dependency). | ||
|
|
||
| `@NonNull` is unnecessary as it is the default. | ||
|
|
||
| **Defensive programming**: Public APIs should still check for `null` parameters even if not | ||
| annotated with `@Nullable`. Internal APIs do not need these checks. | ||
|
|
||
| ### `Optional` usage | ||
|
|
||
| Following the reasoning from | ||
| [Writing a Java library with better experience (slide 12)](https://speakerdeck.com/trustin/writing-a-java-library-with-better-experience?slide=12), | ||
| `java.util.Optional` usage is kept to a minimum. | ||
|
|
||
| **Guidelines**: | ||
|
|
||
| - `Optional` shouldn't appear in public API signatures | ||
| - Avoid `Optional` on the hot path (instrumentation code), unless the instrumented library uses it | ||
|
|
||
| ## Tooling conventions | ||
|
|
||
| ### AssertJ | ||
|
|
||
| Prefer AssertJ assertions over JUnit assertions (assertEquals, assertTrue, etc.) for better error | ||
| messages. | ||
|
|
||
| ### JUnit | ||
|
|
||
| Test classes and test methods should generally be package-protected (no explicit visibility | ||
| modifier) rather than `public`. This follows the principle of minimal necessary visibility and is | ||
| sufficient for JUnit to discover and execute tests. | ||
|
|
||
| ### AutoService | ||
|
|
||
| Use the `@AutoService` annotation when implementing SPI interfaces. This automatically generates the | ||
| necessary `META-INF/services/` files at compile time, eliminating the need to manually create and | ||
| maintain service registration files. | ||
|
|
||
| ```java | ||
| @AutoService(AutoConfigurationCustomizerProvider.class) | ||
| public class MyCustomizerProvider implements AutoConfigurationCustomizerProvider { | ||
| // implementation | ||
| } | ||
| ``` | ||
|
|
||
| ### Gradle | ||
|
|
||
| - Use Kotlin instead of Groovy for build scripts | ||
| - Plugin versions should be specified in `settings.gradle.kts`, not in individual modules | ||
| - All modules should use `plugins { id("otel.java-conventions") }` | ||
| - Set module names with `otelJava.moduleName.set("io.opentelemetry.contrib.mymodule")` | ||
|
|
||
| ## Configuration | ||
|
|
||
| - Use `otel.` prefix for all configuration property keys | ||
| - Read configuration via the `ConfigProperties` interface | ||
| - Provide sensible defaults and document all options | ||
| - Validate configuration early with clear error messages | ||
|
|
||
| ## Performance | ||
|
|
||
| Avoid allocations on the hot path (instrumentation code) whenever possible. This includes `Iterator` | ||
| allocations from collections; note that `for (SomeType t : plainJavaArray)` does not allocate an | ||
| iterator object. | ||
|
|
||
| Non-allocating Stream API usage on the hot path is acceptable but may not fit the surrounding code | ||
| style; this is a judgment call. Some Stream APIs make efficient allocation difficult (e.g., | ||
| `collect` with pre-sized sink data structures involves convoluted `Supplier` code, or lambdas passed | ||
| to `forEach` may be capturing/allocating lambdas). | ||
|
|
||
| ## Documentation | ||
|
|
||
| ### Component README files | ||
|
|
||
| - Include a component owners section in each module's README | ||
| - Document configuration options with examples | ||
|
|
||
| ### Deprecation and breaking changes | ||
|
|
||
| Breaking changes are allowed in unstable modules (published with `-alpha` version suffix). | ||
|
|
||
| 1. Mark APIs with `@Deprecated` and a removal timeline (there must be at least one release with the | ||
| API marked as deprecated before removing it) | ||
| 2. Document the replacement in Javadoc with `@deprecated` tag | ||
| 3. Note the migration path for breaking changes under a "Migration notes" section of CHANGELOG.md | ||
| (create this section at the top of the Unreleased section if not already present) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
[question] do we have an easy way to know when the public API changes in a given PR ?
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.
#2188