Skip to content

Commit

Permalink
Introduce markdown lint check (#7175)
Browse files Browse the repository at this point in the history
Fixes #7129
  • Loading branch information
aaron-ai authored Nov 17, 2022
1 parent 2db0b03 commit 2d7395c
Show file tree
Hide file tree
Showing 67 changed files with 319 additions and 214 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/build-daily.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ jobs:
markdown-link-check:
uses: ./.github/workflows/reusable-markdown-link-check.yml

markdown-lint-check:
uses: ./.github/workflows/reusable-markdown-lint-check.yml

misspell-check:
uses: ./.github/workflows/reusable-misspell-check.yml

Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/build-pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ jobs:
if: "!startsWith(github.ref_name, 'release/')"
uses: ./.github/workflows/reusable-markdown-link-check.yml

# this is a required check to avoid illegal markdown format
markdown-lint-check:
# release branches are excluded to avoid unnecessary maintenance
if: ${{ !startsWith(github.ref_name, 'release/') }}
uses: ./.github/workflows/reusable-markdown-lint-check.yml

# this is not a required check to avoid blocking pull requests if new misspellings are added
# to the misspell dictionary
misspell-check:
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ jobs:
if: "!startsWith(github.ref_name, 'release/')"
uses: ./.github/workflows/reusable-markdown-link-check.yml

markdown-lint-check:
# release branches are excluded
if: ${{ !startsWith(github.ref_name, 'release/') }}
uses: ./.github/workflows/reusable-markdown-lint-check.yml

misspell-check:
# release branches are excluded to avoid unnecessary maintenance if new misspellings are added
# to the misspell dictionary
Expand Down
17 changes: 17 additions & 0 deletions .github/workflows/reusable-markdown-lint-check.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: Reusable - Markdown lint check

on:
workflow_call:

jobs:
markdown-lint-check:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3

- name: Install mardkdownlint
run: npm install -g markdownlint-cli

- name: Run markdownlint
run: |
markdownlint -c .markdownlint.yml -p .gitignore **/*.md -i licenses/licenses.md
14 changes: 14 additions & 0 deletions .markdownlint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# See https://github.com/DavidAnson/markdownlint/blob/main/doc/Rules.md
# and https://github.com/DavidAnson/markdownlint/blob/main/README.md

# Default state for all rules
default: true

ul-style: false
line-length: false
no-duplicate-header:
siblings_only: true
ol-prefix:
style: ordered
no-inline-html: false
fenced-code-language: false
26 changes: 13 additions & 13 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
## Contributing
# Contributing

Pull requests for bug fixes are always welcome!

Before submitting new features or changes to current functionality, it is recommended to first
[open an issue](https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/new)
and discuss your ideas or propose the changes you wish to make.

### Building
## Building

In order to build and test this whole repository you need JDK 11+.

Some instrumentations and tests may put constraints on which java versions they support.
See [Running the tests](./docs/contributing/running-tests.md) for more details.

#### Snapshot builds
### 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 OSS snapshots repository at `https://oss.sonatype.org/content/repositories/snapshots/`
([browse](https://oss.sonatype.org/content/repositories/snapshots/io/opentelemetry/))

#### Building from source
### Building from source

Build using Java 11+:

Expand All @@ -36,39 +36,39 @@ and then you can find the java agent artifact at

`javaagent/build/libs/opentelemetry-javaagent-<version>.jar`.

### IntelliJ setup and troubleshooting
## IntelliJ setup and troubleshooting

See [IntelliJ setup and troubleshooting](docs/contributing/intellij-setup-and-troubleshooting.md)

### Style guide
## Style guide

See [Style guide](docs/contributing/style-guideline.md)

### Running the tests
## Running the tests

See [Running the tests](docs/contributing/running-tests.md)

### Writing instrumentation
## Writing instrumentation

See [Writing instrumentation](docs/contributing/writing-instrumentation.md)

### Understanding the javaagent structure
## Understanding the javaagent structure

See [Understanding the javaagent structure](docs/contributing/javaagent-structure.md)

### Understanding the javaagent instrumentation testing components
## Understanding the javaagent instrumentation testing components

See [Understanding the javaagent instrumentation testing components](docs/contributing/javaagent-test-infra.md)

### Debugging
## Debugging

See [Debugging](docs/contributing/debugging.md)

### Understanding Muzzle
## Understanding Muzzle

See [Understanding Muzzle](docs/contributing/muzzle.md)

### Troubleshooting PR build failures
## Troubleshooting PR build failures

The build logs are very long and there is a lot of parallelization, so the logs can be hard to
decipher, but if you scroll to the bottom you should see something like:
Expand Down
6 changes: 3 additions & 3 deletions VERSIONING.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
## Compatibility requirements

Artifacts in this repository follow the same compatibility requirements described in
https://github.com/open-telemetry/opentelemetry-java/blob/main/VERSIONING.md#compatibility-requirements
<https://github.com/open-telemetry/opentelemetry-java/blob/main/VERSIONING.md#compatibility-requirements>
.

EXCEPT for the following incompatible changes which are allowed in stable artifacts in this
repository:

* Changes to the telemetry produced by instrumentation
(there will be some guarantees about telemetry stability in the future, see discussions
in https://github.com/open-telemetry/opentelemetry-specification/issues/1301)
in <https://github.com/open-telemetry/opentelemetry-specification/issues/1301>)
* Changes to configuration properties that contain the word `experimental`
* Changes to configuration properties under the namespace `otel.javaagent.testing`

Expand All @@ -23,7 +23,7 @@ This means that:

## Stable vs alpha

See https://github.com/open-telemetry/opentelemetry-java/blob/main/VERSIONING.md#stable-vs-alpha
See <https://github.com/open-telemetry/opentelemetry-java/blob/main/VERSIONING.md#stable-vs-alpha>

IN PARTICULAR:

Expand Down
7 changes: 5 additions & 2 deletions benchmark-overhead/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* [Process](#process)
* [What do we measure?](#what-do-we-measure)
* [Config](#config)
+ [Agents](#agents)
* [Agents](#agents)
* [Automation](#automation)
* [Setup and Usage](#setup-and-usage)
* [Visualization](#visualization)
Expand All @@ -23,9 +23,10 @@ There is one dynamic test here called [OverheadTests](https://github.com/open-te
The `@TestFactory` method creates a test pass for each of the [defined configurations](https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/benchmark-overhead/src/test/java/io/opentelemetry/config/Configs.java).
Before the tests run, a single collector instance is started. Each test pass has one or more agents configured and those are tested in series.
For each agent defined in a configuration, the test runner (using [testcontainers](https://www.testcontainers.org/)) will:

1. create a fresh postgres instance and populate it with initial data.
2. create a fresh instance of [spring-petclinic-rest](https://github.com/spring-petclinic/spring-petclinic-rest) instrumented with the specified agent
3. measure the time until the petclinic app is marked "healthy" and then write it to a file
3. measure the time until the petclinic app is marked "healthy" and then write it to a file.
4. if configured, perform a warmup phase. During the warmup phase, a bit of traffic is generated in order to get the application into a steady state (primarily helping facilitate jit compilations). Currently, we use a 30 second warmup time.
5. start a JFR recording by running `jcmd` inside the petclinic container
6. run the [k6 test script](https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/benchmark-overhead/k6/basic.js) with the configured number of iterations through the file and the configured number of concurrent virtual users (VUs).
Expand Down Expand Up @@ -65,6 +66,7 @@ relative overhead.
## Config

Each config contains the following:

* name
* description
* list of agents (see below)
Expand All @@ -74,6 +76,7 @@ Each config contains the following:
* warmupSeconds - how long to wait before starting conducting measurements

Currently, we test:

* no agent versus latest released agent
* no agent versus latest snapshot
* latest release vs. latest snapshot
Expand Down
8 changes: 4 additions & 4 deletions docs/contributing/debugging.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
## Debugging
# Debugging

Debugging java agent can be a challenging task since some instrumentation
code is directly inlined into target classes.

### Advice methods
## Advice methods

Breakpoints do not work in advice methods, because their code is directly inlined
by ByteBuddy into the target class. It is good to keep these methods as small as possible.
Expand All @@ -21,13 +21,13 @@ System.out.println()
Thread.dumpStack()
```

### Agent initialization code
## Agent initialization code

If you want to debug agent initialization code (e.g. `OpenTelemetryAgent`, `AgentInitializer`,
`AgentInstaller`, `OpenTelemetryInstaller`, etc.) then it's important to specify the `-agentlib:` JVM arg
before the `-javaagent:` JVM arg and use `suspend=y` (see full example below).

### Enabling debugging
## Enabling debugging

The following example shows remote debugger configuration. The breakpoints
should work in any code except ByteBuddy advice methods.
Expand Down
2 changes: 1 addition & 1 deletion docs/contributing/intellij-setup-and-troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Configuration:
![enable google format](https://user-images.githubusercontent.com/5099946/131759832-36437aa0-a5f7-42c0-9425-8c5b45c16765.png)

Note: If google-java-format generates errors in Intellij,
see https://github.com/google/google-java-format/issues/787#issuecomment-1200762464.
see <https://github.com/google/google-java-format/issues/787#issuecomment-1200762464>.

## Troubleshooting

Expand Down
4 changes: 4 additions & 0 deletions docs/contributing/muzzle.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Muzzle will prevent loading an instrumentation if it detects any mismatch or con
## How it works

Muzzle has two phases:

* at compile time it collects references to the third-party symbols and used helper classes;
* at runtime it compares those references to the actual API symbols on the classpath.

Expand Down Expand Up @@ -73,12 +74,15 @@ it's not an optional feature.
The gradle plugin defines two tasks:

* `muzzle` task runs the runtime muzzle verification against different library versions:

```sh
./gradlew :instrumentation:google-http-client-1.19:javaagent:muzzle
```

If a new, incompatible version of the instrumented library is published it fails the build.

* `printMuzzleReferences` task prints all API references in a given module:

```sh
./gradlew :instrumentation:google-http-client-1.19:javaagent:printMuzzleReferences
```
Expand Down
2 changes: 1 addition & 1 deletion docs/contributing/repository-settings.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Repository settings

Repository settings in addition to what's documented already at
https://github.com/open-telemetry/community/blob/main/docs/how-to-configure-new-repository.md.
<https://github.com/open-telemetry/community/blob/main/docs/how-to-configure-new-repository.md>.

## General > Pull Requests

Expand Down
2 changes: 1 addition & 1 deletion docs/contributing/using-instrumenter-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ In some rare cases it may be useful to completely disable the constructed `Instr
example, based on a configuration property. The `InstrumenterBuilder` exposes a `setEnabled()`
method for that: passing `false` will turn the newly created `Instrumenter` into a no-op instance.

### Finally, set the span kind with the `SpanKindExtractor` and get a new `Instrumenter`!
### Finally, set the span kind with the `SpanKindExtractor` and get a new `Instrumenter`

The `Instrumenter` creation process ends with calling one of the following `InstrumenterBuilder`
methods:
Expand Down
4 changes: 4 additions & 0 deletions docs/contributing/writing-instrumentation-module.md
Original file line number Diff line number Diff line change
Expand Up @@ -344,14 +344,18 @@ compile time for it to work.
### Why we don't use ByteBuddy @Advice.Origin Method

Instead of

```
@Advice.Origin Method method
```

we prefer to use

```
@Advice.Origin("#t") Class<?> declaringClass,
@Advice.Origin("#m") String methodName
```

because the former inserts a call to `Class.getMethod(...)` in transformed method. In contrast,
getting the declaring class and method name is just loading constants from constant pool, which is
a much simpler operation.
Expand Down
1 change: 1 addition & 0 deletions docs/contributing/writing-instrumentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ That way you can use these shared, globally available utilities to communicate b
instrumentation modules.

Some examples of this include:

* Application server instrumentations communicating with Servlet API instrumentations.
* Different high-level Kafka consumer instrumentations suppressing the low-level `kafka-clients`
instrumentation.
Expand Down
16 changes: 8 additions & 8 deletions docs/java-7-rationale.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
## Rationale for not supporting Java 7
# Rationale for not supporting Java 7

### Android support is no longer tied to Java 7
## Android support is no longer tied to Java 7

Even for supporting old Android API levels:

> If you're building your app using Android Gradle plugin 4.0.0 or higher, the plugin extends
> support for using a number of Java 8 language APIs <b>without requiring a minimum API level for
> your app</b>.
(https://developer.android.com/studio/write/java8-support#library-desugaring)
(<https://developer.android.com/studio/write/java8-support#library-desugaring>)

There are some Java 8 APIs that Android does not desugar, but we can use
[animal sniffer plugin](https://github.com/xvik/gradle-animalsniffer-plugin) to ensure we don't use
Expand All @@ -20,11 +20,11 @@ We will use this approach for the `instrumentation-api` module and for any libra
instrumentation that would be useful to Android developers
(e.g. library instrumentation for OkHttp).

### Modern test tooling requires Java 8+
## Modern test tooling requires Java 8+

Both JUnit 5 and Testcontainers require Java 8+.

### Auto-instrumentation (Javaagent)
## Auto-instrumentation (Javaagent)

We could run tests against Java 8+ and ensure Java 7 compliance by using similar animal sniffer
technique as above.
Expand All @@ -43,21 +43,21 @@ debugging across two separate JVMs. And new contributor experience has a very hi
project (compared to say commercial tools who can invest more in onboarding their employees onto a
more complex codebase).

### Library (manual) instrumentation
## Library (manual) instrumentation

We believe that Java 7 users are primarily in maintenance mode and not interested in cracking open
their code anymore and adding library (manual) instrumentation, so we don't believe there is much
interest in library instrumentation targeting Java 7.

### Java 7 usage
## Java 7 usage

Certainly one factor to consider is what percentage of production applications are running Java 7.

Luckily, New Relic
[published their numbers recently](https://blog.newrelic.com/technology/state-of-java),
so we know that ~2.5% of production applications are still running Java 7 as of March 2020.

### Alternatives for Java 7 users
## Alternatives for Java 7 users

We understand the situations that lead applications to get stuck on Java 7 (we've been there
ourselves), and we agree that those applications need monitoring too.
Expand Down
1 change: 1 addition & 0 deletions docs/logger-mdc-instrumentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Those three pieces of information can be included in log statements produced by
by specifying them in the pattern/format.

Tip: for Spring Boot configuration which uses logback, you can add MDC to log lines by overriding only the `logging.pattern.level`:

```properties
logging.pattern.level = trace_id=%mdc{trace_id} span_id=%mdc{span_id} trace_flags=%mdc{trace_flags} %5p
```
Expand Down
Loading

0 comments on commit 2d7395c

Please sign in to comment.