Skip to content

Commit f476295

Browse files
committed
refactor(core,instr,agent,client): remove permission API and update instrumentation/tests
Removes legacy permission annotations and extension permission markers. Adjusts core extension metadata, messages, and dtrace integration. Updates instrumentation pipeline and verifiers; refreshes related tests and test data. Touches client/agent entrypoints and messaging; polishes samples and README. Build tweaks across modules and CI workflow.
1 parent 20479dc commit f476295

257 files changed

Lines changed: 13468 additions & 3942 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/continuous.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ jobs:
131131
- name: Run tests
132132
run: |
133133
set +x
134-
./gradlew --no-daemon --parallel --build-cache -Pintegration :integration-tests:test
134+
./gradlew --no-daemon --parallel --build-cache -Pintegration -PCI :integration-tests:test
135135
- name: Check Gradle cache size
136136
run: du -sh ~/.gradle/caches
137137
- name: Integration test reports

CONTRIBUTING.md

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
# Contributing to BTrace
2+
3+
Thanks for your interest in contributing! This guide covers local development, running tests, Gradle tips, and common troubleshooting.
4+
5+
Note: Pull requests can only be accepted from signers of the Oracle Contributor Agreement (OCA). See the project README for details.
6+
7+
## Local Development
8+
9+
- JDK: Use a reasonably recent JDK (11+ recommended). The project targets a broad range but tests run comfortably on 11/17.
10+
- Wrapper: Use the bundled `./gradlew` wrapper. It will download the pinned Gradle version if needed.
11+
- Local Gradle cache (optional but recommended):
12+
- macOS/Linux: `export GRADLE_USER_HOME="$PWD/.gradle-home"`
13+
- Windows (PowerShell): `$env:GRADLE_USER_HOME = "$PWD/.gradle-home"`
14+
15+
## Running Tests
16+
17+
- All unit tests (skip integration tests):
18+
```sh
19+
./gradlew --no-daemon test -x integration-tests:test
20+
```
21+
22+
- Per-module tests:
23+
- Runtime: `./gradlew :btrace-runtime:test`
24+
- Extension: `./gradlew :btrace-extension:test`
25+
- Compiler: `./gradlew :btrace-compiler:test`
26+
- Instr: `./gradlew :btrace-instr:test`
27+
28+
- Update instrumentor goldens when bytecode output changes:
29+
```sh
30+
./gradlew test -PupdateTestData
31+
```
32+
33+
- Integration tests (spawn JVMs, exercise agent and extensions):
34+
```sh
35+
./gradlew --no-daemon integration-tests:test
36+
```
37+
- If tests fail due to denied privileged extensions, pass a policy file to the tested JVMs:
38+
- Create `permissions.properties`:
39+
```properties
40+
allowPrivileged=true
41+
allowExtensions=btrace-metrics,btrace-utils
42+
```
43+
- Export path: `export BTRACE_PERMS=$PWD/permissions.properties`
44+
- Run Gradle with: `-Dbtrace.permissions=$BTRACE_PERMS`
45+
46+
## Gradle Tips
47+
48+
- Prefer IPv4 if your environment has unusual local IP settings (helps Gradle select a wildcard address):
49+
```sh
50+
export GRADLE_OPTS="-Djava.net.preferIPv4Stack=true -Djava.net.preferIPv6Addresses=false"
51+
```
52+
- Enable Gradle debug output for flakiness: add `--info` or `--debug`.
53+
- Run a single test class/method:
54+
```sh
55+
./gradlew :btrace-extension:test --tests org.openjdk.btrace.extension.ExtensionBridgeImplPolicyTest
56+
./gradlew :btrace-runtime:test --tests "*ExtensionIndyShimIndexTest.resolvesNoopShimFromIndex"
57+
```
58+
59+
## Troubleshooting
60+
61+
- Gradle wrapper needs to download Gradle: ensure network is allowed once; subsequent runs use the local cache under `.gradle-home`.
62+
- Error: `Could not determine a usable wildcard IP for this machine`:
63+
- Set the IPv4 flags shown above or ensure local networking is available.
64+
- Permission errors when Gradle writes outside the workspace:
65+
- Use a local Gradle cache via `GRADLE_USER_HOME` as shown above.
66+
- Integration tests failing with permissions denied:
67+
- Provide a policy file and pass it via `-Dbtrace.permissions=/path/to/permissions.properties`.
68+
69+
## Code Style & Scope
70+
71+
- Keep changes focused and minimal; align with existing code style.
72+
- Update docs when changing user-visible behavior.
73+
- Prefer clear separation of concerns and small helpers over inlined, complex logic.
74+
- Avoid introducing new dependencies without discussion.
75+
76+
## Submitting a PR
77+
78+
1. Fork the repo and branch from `develop` (unless otherwise agreed).
79+
2. Make your changes and run tests locally.
80+
3. If instrumentor behavior changed, update goldens (`-PupdateTestData`) and include them in your commit.
81+
4. Submit a PR with a concise description of the change, rationale, and any follow-ups.
82+
83+
Happy tracing!
84+

README.md

Lines changed: 66 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,30 @@ When instrumentor code changes, update test golden files with:
4242
```
4343
Commit the regenerated golden files to Git.
4444

45+
### How To Run Tests
46+
47+
For a fast local cycle, run unit tests and skip integration tests (which start forked JVMs):
48+
49+
```sh
50+
export GRADLE_USER_HOME="$PWD/.gradle-home" # keep caches inside the repo (optional)
51+
./gradlew --no-daemon test -x integration-tests:test
52+
```
53+
54+
Tips:
55+
- Prefer IPv4 if your environment has odd local IPs: set `GRADLE_OPTS="-Djava.net.preferIPv4Stack=true -Djava.net.preferIPv6Addresses=false"`.
56+
- Run specific modules:
57+
- Runtime: `./gradlew :btrace-runtime:test`
58+
- Extension: `./gradlew :btrace-extension:test`
59+
- Compiler: `./gradlew :btrace-compiler:test`
60+
- Instr: `./gradlew :btrace-instr:test`
61+
- Update instrumentor golden files when bytecode output changes: `./gradlew test -PupdateTestData`.
62+
63+
Integration tests (optional):
64+
```sh
65+
./gradlew --no-daemon integration-tests:test
66+
```
67+
These may exercise privileged extensions. If you run into permission denials, provide a policy file and pass it to the test JVMs via `-Dbtrace.permissions=/path/to/permissions.properties`.
68+
4569

4670
## Using BTrace
4771

@@ -188,12 +212,7 @@ BTrace supports extensions (like StatsdExtension) that provide additional functi
188212
* **Standard permissions** (granted unless denied): FILE_READ, SYSTEM_PROPS, THREAD_INFO, MEMORY_INFO
189213
* **Privileged permissions** (require explicit grant): FILE_WRITE, NETWORK, THREADS, NATIVE, EXEC, REFLECTION, CLASSLOADER, UNLIMITED_MEMORY
190214

191-
Declare required permissions in your probe with `@RequestPermission`:
192-
```java
193-
@BTrace
194-
@RequestPermission(Permission.NETWORK)
195-
public class MyProbe { ... }
196-
```
215+
Permissions are enforced based on extension/service descriptors and agent grants specified at attach-time.
197216

198217
Grant permissions at runtime:
199218
```sh
@@ -207,6 +226,47 @@ btrace -le <PID>
207226

208227
See the [Tutorial](docs/BTraceTutorial.md) for detailed documentation.
209228

229+
Extensions CLI: use `btracex` to inspect and manage extensions and the simplified permission policy:
230+
- `btracex inspect <zip|dir>` prints extension id, version, services, and whether it’s privileged.
231+
- `btracex policy print|set [--policy-file <path>|--home|--classpath <outDir>]` edits `allowExtensions`, `denyExtensions`, `allowPrivileged`.
232+
- `btracex list` shows installed extensions; `btracex install` installs from Maven coordinates.
233+
234+
Note: Extension “required permissions” are informational and help operators assess risk. Implementation linking is controlled by per‑extension allow/deny lists and the `allowPrivileged` flag; when blocked, APIs remain available and SHIMs are used so probes continue safely.
235+
236+
#### Agent Policy and Allow/Deny Lists
237+
- Launch-time policy can be set via agent args (operator-controlled):
238+
- `-javaagent:btrace-agent.jar=...,grant=NETWORK,THREADS,grantAll=false`
239+
- `-javaagent:btrace-agent.jar=...,allowExtensions=btrace-statsd,my-metrics,denyExtensions=legacy-foo`
240+
- Optional policy file (process-local): `-Dbtrace.permissions=/path/to/permissions.properties` or `~/.btrace/permissions.properties`.
241+
- When an extension impl is blocked, the API remains on bootstrap so SHIMs can be generated.
242+
243+
See docs/PermissionPolicy.md for details and examples.
244+
245+
#### btracex TUI (interactive)
246+
- Launch: `btracex inspect` (with no args) opens an interactive view of installed extensions.
247+
- Header: shows current policy file path and the list of scanned repositories.
248+
- Table: columns State, Id, Version. State uses compact symbols: `?` (default), `+` (allowed), `-` (denied).
249+
- Details: selection updates automatically; shows the full-word state: `default` / `allowed` / `denied` and the full path.
250+
- Legend: a short legend under the table maps the state symbols.
251+
252+
Screenshot / demo (optional):
253+
254+
![btracex TUI](docs/images/btracex-tui.png)
255+
256+
![btracex TUI demo](docs/images/btracex-tui.gif)
257+
258+
See also: docs/TUI.md for recording tips and an ASCII preview.
259+
260+
Keys
261+
- Navigate: Arrow keys, PageUp/PageDown, Home/End
262+
- Toggle state: space (flows `? → + (confirm) → - → +`; only `c` clears to default)
263+
- Clear: `c` (removes extension id from both allow and deny lists)
264+
- Explain privileges: `e` (opens a dialog with required permissions and risk descriptions)
265+
- Filter: `/` (filter by id or path)
266+
- Sort: `s` (choose column; repeat to toggle asc/desc)
267+
- Adjust split: `m` (enter mode), then Up/Down to resize; press `Esc` or `m` again to exit
268+
- Help / Quit: `?` / `q`
269+
210270
### Maven Integration
211271

212272
The [BTrace Maven Plugin](https://github.com/btraceio/btrace-maven) enables:

benchmarks/runtime-benchmarks/build.gradle

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ dependencies {
1717
implementation project(":btrace-compiler")
1818
jmh project(":btrace-instr")
1919
jmh project(":btrace-runtime")
20-
jmh project(":btrace-statsd")
20+
jmh project(":btrace-extensions:btrace-statsd")
2121
jmh libs.jmh
2222
jmh libs.jmh.annprocess
2323
compilerDeps project(path: ":btrace-dist", configuration: "shadow")
@@ -52,7 +52,7 @@ jmhJar {
5252
include "org/openjdk/btrace/instr/**"
5353
include 'org/openjdk/btrace/generated/**/*'
5454
include 'org/openjdk/btrace/runtime/**'
55-
include 'org/openjdk/btrace/services/**'
55+
// no legacy services classes to package
5656
include "joptsimple/**"
5757
include "org/apache/**"
5858
include '*.btclass'
@@ -66,4 +66,3 @@ jmh {
6666
includes = ['org.openjdk.btrace.bench.ClassFilterBenchmark']
6767
verbosity = 'EXTRA'
6868
}
69-

btrace-agent/build.gradle

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,12 @@ dependencies {
2121
implementation files("${toolsJar}")
2222
}
2323
implementation project(':btrace-core')
24-
implementation project(':btrace-services-api')
2524
implementation project(':btrace-runtime')
2625
implementation project(':btrace-instr')
27-
}
26+
implementation project(':btrace-extension')
27+
}
28+
29+
// Exclude sources that rely on JDK-internal modules from Javadoc to avoid missing-package errors
30+
tasks.named('javadoc').configure {
31+
exclude 'org/openjdk/btrace/agent/PerfReaderImpl.java'
32+
}

btrace-agent/src/main/java/org/openjdk/btrace/agent/Client.java

Lines changed: 87 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@
4141
import org.openjdk.btrace.core.comm.StatusCommand;
4242
import org.openjdk.btrace.core.extensions.Permission;
4343
import org.openjdk.btrace.core.extensions.PermissionSet;
44+
import org.openjdk.btrace.extension.ExtensionDescriptorDTO;
45+
import org.openjdk.btrace.extension.ExtensionLoader;
46+
import org.openjdk.btrace.extension.ExtensionRegistry;
4447
import org.openjdk.btrace.instr.BTraceProbe;
4548
import org.openjdk.btrace.instr.BTraceProbeFactory;
4649
import org.openjdk.btrace.instr.BTraceProbePersisted;
@@ -54,7 +57,6 @@
5457
import org.openjdk.btrace.instr.MethodTrackingContext;
5558
import org.openjdk.btrace.runtime.BTraceRuntimeAccess;
5659
import org.openjdk.btrace.runtime.BTraceRuntimes;
57-
import org.openjdk.btrace.runtime.ExtensionRegistry;
5860
import org.slf4j.Logger;
5961
import org.slf4j.LoggerFactory;
6062

@@ -348,6 +350,9 @@ final Class<?> loadClass(InstrumentCommand instr) throws IOException {
348350
throw new SecurityException(formatPermissionError(missing));
349351
}
350352
}
353+
354+
// Validate that all injected service types are declared by available extensions
355+
validateDeclaredServices(probe);
351356
} catch (Throwable th) {
352357
log.debug("Failed to load BTrace probe code", th);
353358
errorExit(th);
@@ -393,6 +398,26 @@ final Class<?> loadClass(InstrumentCommand instr) throws IOException {
393398
sendCommand(new MessageCommand(warning.toString()));
394399
}
395400

401+
// Expose extension-declared permissions for integration visibility
402+
// Print extension permissions only when explicitly requested (debug or system property)
403+
if (settings.isDebug() || Boolean.getBoolean("btrace.list.extension.permissions")) {
404+
try {
405+
ExtensionLoader loader = Main.getExtensionLoader();
406+
if (loader != null) {
407+
StringBuilder info = new StringBuilder();
408+
info.append("[BTRACE INFO] Extensions and declared permissions:\n");
409+
for (ExtensionDescriptorDTO ext : loader.getAvailableExtensions()) {
410+
PermissionSet perms = ext.getRequiredPermissions();
411+
String pStr = perms != null && !perms.isEmpty() ? perms.toString() : "[]";
412+
info.append(" - ").append(ext.getId()).append(": ").append(pStr).append("\n");
413+
}
414+
sendCommand(new MessageCommand(info.toString()));
415+
}
416+
} catch (Throwable t) {
417+
// ignore, informational only
418+
}
419+
}
420+
396421
boolean entered = false;
397422
try {
398423
entered = BTraceRuntimeAccess.enter(runtime);
@@ -408,6 +433,62 @@ final Class<?> loadClass(InstrumentCommand instr) throws IOException {
408433
}
409434
}
410435

436+
/**
437+
* Validates that all {@code @Injected} service field types used by the given probe are
438+
* declared by some available extension. This runs in the agent's runtime where the actual
439+
* classloader and JPMS module layer apply.
440+
*
441+
* Why reflection here (vs. pure ASM):
442+
* - Classloader identity: Ensures types are checked against the agent's classes loaded by the
443+
* correct loader. Name-only checks in ASM cannot detect split-brain issues (same FQN, different
444+
* loader/JAR) that would later cause ClassCastException.
445+
* - JPMS access rules: Surfaces missing exports/opens and other module access constraints that
446+
* cannot be proven by static bytecode analysis.
447+
* - Linkage/loadability: Fails fast if a referenced type is not actually resolvable on the
448+
* agent's runtime path (NoClassDefFoundError/missing transitive dependencies).
449+
* - Assignability truth: Verifies that the service type corresponds to something an extension
450+
* actually declares in its manifest, avoiding false positives from shaded or version-skewed
451+
* classes.
452+
*
453+
* Implementation notes:
454+
* - We use reflection only to access the probe's internal service field map (to avoid a direct
455+
* compile-time dependency on the probe's delegate type) and to keep the agent/probe boundary
456+
* clean. We do not instantiate user classes or trigger class initializers.
457+
* - This check complements compile-time and bytecode-time validation (ASM-based) which enforce
458+
* structural rules without loading classes. Reflection here provides the necessary runtime
459+
* assurance in the actual environment where the agent will operate.
460+
*/
461+
private void validateDeclaredServices(BTraceProbe probe) throws IOException {
462+
if (!(probe instanceof BTraceProbePersisted)) {
463+
return;
464+
}
465+
ExtensionLoader loader = Main.getExtensionLoader();
466+
if (loader == null) {
467+
return;
468+
}
469+
// Reflectively access serviceFields() from the delegate to get injected service types
470+
try {
471+
java.lang.reflect.Field delF = BTraceProbePersisted.class.getDeclaredField("delegate");
472+
delF.setAccessible(true);
473+
Object delegate = delF.get(probe);
474+
java.lang.reflect.Method svcM = delegate.getClass().getDeclaredMethod("serviceFields");
475+
svcM.setAccessible(true);
476+
@SuppressWarnings("unchecked")
477+
Map<String, String> svcMap = (Map<String, String>) svcM.invoke(delegate);
478+
if (svcMap != null) {
479+
for (String internalName : svcMap.values()) {
480+
String fqcn = internalName.replace('/', '.');
481+
if (loader.findExtensionForService(fqcn) == null) {
482+
throw new IOException(
483+
"Injected service type not declared by any extension: " + fqcn);
484+
}
485+
}
486+
}
487+
} catch (ReflectiveOperationException e) {
488+
log.debug("Unable to inspect injected services for validation", e);
489+
}
490+
}
491+
411492
protected void closeAll() throws IOException {
412493
if (flusher != null) {
413494
flusher.cancel();
@@ -521,7 +602,8 @@ boolean retransformLoaded() throws UnmodifiableClassException {
521602
log.debug("Attempting to retransform class: {}", c.getName());
522603
inst.retransformClasses(c);
523604
} catch (ClassFormatError | VerifyError e) {
524-
log.debug("Class '{}' verification failed", c.getName(), e);
605+
// Avoid printing full stack traces in debug to keep target stderr clean
606+
log.debug("Class '{}' verification failed: {}", c.getName(), e.toString());
525607
sendCommand(
526608
new MessageCommand(
527609
"[BTRACE WARN] Class verification failed: "
@@ -544,13 +626,14 @@ boolean retransformLoaded() throws UnmodifiableClassException {
544626
try {
545627
inst.retransformClasses(c);
546628
} catch (ClassFormatError | VerifyError e1) {
547-
log.debug("Class '{}' verification failed", c.getName(), e);
629+
// Avoid printing full stack traces in debug to keep target stderr clean
630+
log.debug("Class '{}' verification failed: {}", c.getName(), e1.toString());
548631
sendCommand(
549632
new MessageCommand(
550633
"[BTRACE WARN] Class verification failed: "
551634
+ c.getName()
552635
+ " ("
553-
+ e.getMessage()
636+
+ e1.getMessage()
554637
+ ")"));
555638
}
556639
}

0 commit comments

Comments
 (0)