Skip to content

fix(exposition): export internal package for OSGi resolution #2415

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.

Sign up for GitHub

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

Open
arimu1 wants to merge 2 commits into
base: main
Choose a base branch
from
+30 −0
Open

fix(exposition): export internal package for OSGi resolution #2415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
687e885
fix(exposition): export internal package for OSGi resolution
arimu1 Aug 24, 2026
991921d
Merge branch 'main' into fix/2395-osgi-expositionformats-internal
jaydeluca Aug 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions prometheus-metrics-exposition-formats-shaded/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,21 @@

<build>
<plugins>
<plugin>
<groupId>org.apache.felix</groupId>
<artifactId>maven-bundle-plugin</artifactId>
<configuration>
<instructions>
<!-- Required for OSGi: textformats loads protobuf impl via Class.forName -->
<Export-Package>
io.prometheus.metrics.expositionformats.generated*;version="${project.version}"
</Export-Package>
<_exportcontents>
io.prometheus.metrics.expositionformats.internal;version="${project.version}"
</_exportcontents>
Comment on lines +70 to +75

@jaydeluca jaydeluca Aug 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

${project.version} is 1.8.1-SNAPSHOT on snapshot builds, which isn't valid OSGi version
syntax, so bnd rewrites it to 1.8.1.SNAPSHOT. That sorts below a released 1.8.1. Built both
branches to confirm the delta:

main:   io.prometheus.metrics.expositionformats.generated;version="1.8.1"
branch: io.prometheus.metrics.expositionformats.generated;version="1.8.1.SNAPSHOT"

bnd already defaults exported-package versions to the cleaned project version, so on a release
build the attribute produces exactly what you'd get without it — it only changes behavior on
snapshots, and only for the worse. Dropping it restores 1.8.1 (verified locally).

Suggested change
<Export-Package>
io.prometheus.metrics.expositionformats.generated*;version="${project.version}"
</Export-Package>
<_exportcontents>
io.prometheus.metrics.expositionformats.internal;version="${project.version}"
</_exportcontents>
<Export-Package>
io.prometheus.metrics.expositionformats.generated*
</Export-Package>
<_exportcontents>
io.prometheus.metrics.expositionformats.internal
</_exportcontents>

Same applies to prometheus-metrics-exposition-formats-shaded/pom.xml

</instructions>

@jaydeluca jaydeluca Aug 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While verifying this fix I hit a second manifest problem in this bundle that's pre-existing, but
it matters for whether #2395 is actually resolved.

maven-bundle-plugin runs before maven-shade-plugin, so bnd computes the manifest against
unrelocated classes. On both main and this branch the shaded bundle declares:

Import-Package: com.google.protobuf;version="[4.36,5)"   ← mandatory

…while the jar contains zero com/google/protobuf/ classes. I decompiled
internal/ProtobufUtil.class out of the built shaded jar and it references
io/prometheus/metrics/shaded/com_google_protobuf_4_36_0/*, as you'd expect. So the shaded
artifact demands a real protobuf-java bundle be installed, which rather defeats the point of
shading.

Why this blocks the original report: micrometer-registry-prometheus depends on
io.prometheus:prometheus-metrics-exposition-formats — the shaded artifact — at runtime scope. So
the bnd resolve in #2395 will get past internal after this fix and then stop at
com.google.protobuf unless protobuf-java happens to be in their resolution repository.

Second-order effect from this PR specifically: the newly exported internal package inherits
uses:="com.google.protobuf,...", propagating the bogus constraint to every consumer that wires
to it.

Adding !com.google.protobuf removes both the phantom import and the bad uses clause. Verified
against a local build:

Suggested change
</instructions>
<Import-Package>!com.google.protobuf,*</Import-Package>
</instructions>

Resulting manifest:

Export-Package: ...internal;version="1.8.1";uses:="io.prometheus.metrics.config,..."
                                              ↑ no com.google.protobuf
Import-Package: io.prometheus.metrics.config;version="[1.8,2)",...
                ↑ com.google.protobuf gone

with all that being said, are there some tests we can include to verify that all of this actually works and fixes the issue? I have limited experience with OSGi, so I defer to you and @anjeongkyun to hopefully weigh in

</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-resources-plugin</artifactId>
Expand Down
15 changes: 15 additions & 0 deletions prometheus-metrics-exposition-formats/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,21 @@

<build>
<plugins>
<plugin>
<groupId>org.apache.felix</groupId>
<artifactId>maven-bundle-plugin</artifactId>
<configuration>
<instructions>
<!-- Required for OSGi: textformats loads protobuf impl via Class.forName -->

@jaydeluca jaydeluca Aug 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is the right diagnosis, and it points at the half of the problem that this PR doesn't fix.

createProtobufWriter() is explicitly written to tolerate the impl being absent — it catches, returns null, and isAvailable() reports false (prometheus-metrics-exposition-textformats/.../PrometheusProtobufWriter.java:26-37). But bnd emits the import as mandatory:

Import-Package: ... io.prometheus.metrics.expositionformats.internal   ← no resolution:=optional

So a deployment that installs only prometheus-metrics-exposition-textformats — which is the
entire reason that artifact ships separately from the protobuf-carrying ones — still fails to
resolve after this change. Exporting internal fixes the case where the formats bundle is
present; it doesn't fix the case where it deliberately isn't.

One line in prometheus-metrics-exposition-textformats/pom.xml, in a maven-bundle-plugin
<instructions> block mirroring the ones you've added here:

<Import-Package>io.prometheus.metrics.expositionformats.internal;resolution:=optional,*</Import-Package>

I built this and confirmed the manifest:

Import-Package: io.prometheus.metrics.expositionformats.internal;resolution:=optional,
                io.prometheus.metrics.config;version="[1.8,2)",...

@jaydeluca jaydeluca Aug 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, the comment sits above <Export-Package> but explains <_exportcontents>

<Export-Package>
io.prometheus.metrics.expositionformats.generated*;version="${project.version}"
</Export-Package>
<_exportcontents>
io.prometheus.metrics.expositionformats.internal;version="${project.version}"
</_exportcontents>
</instructions>
</configuration>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>build-helper-maven-plugin</artifactId>
Expand Down