Netflix / spectator

Client library for collecting metrics.
Apache License 2.0
741 stars 168 forks source link

spectator-reg-atlas POM file has wrong packaging type since 1.7.5 #1113

Closed brharrington closed 7 months ago

brharrington commented 7 months ago

It was inadvertently changed to <packaging>pom</packaging> in 1.7.5 causing some problems for some projects trying to pull it in, for example https://github.com/micrometer-metrics/micrometer/pull/4700.

/home/********/micrometer/implementations/micrometer-registry-atlas/src/main/java/io/micrometer/atlas/AtlasMeterRegistry.java:24: error: package com.netflix.spectator.atlas does not exist
import com.netflix.spectator.atlas.AtlasConfig;
                                  ^
/home/********/micrometer/implementations/micrometer-registry-atlas/src/main/java/io/micrometer/atlas/AtlasMeterRegistry.java:25: error: package com.netflix.spectator.atlas does not exist
import com.netflix.spectator.atlas.AtlasRegistry;
                                  ^
/home/********/micrometer/implementations/micrometer-registry-atlas/src/main/java/io/micrometer/atlas/AtlasMeterRegistry.java:51: error: cannot find symbol
    private final AtlasRegistry registry;
                  ^
  symbol:   class AtlasRegistry
  location: class AtlasMeterRegistry
/home/********/micrometer/implementations/micrometer-registry-atlas/src/main/java/io/micrometer/atlas/AtlasMeterRegistry.java:53: error: cannot find symbol
    private final AtlasConfig atlasConfig;
                  ^
  symbol:   class AtlasConfig
  location: class AtlasMeterRegistry
/home/********/micrometer/implementations/micrometer-registry-atlas/src/main/java/io/micrometer/atlas/AtlasMeterRegistry.java:55: error: cannot find symbol
    public AtlasMeterRegistry(AtlasConfig config, Clock clock) {
                              ^
  symbol:   class AtlasConfig
  location: class AtlasMeterRegistry
/home/********/micrometer/implementations/micrometer-registry-atlas/src/main/java/io/micrometer/atlas/AtlasMeterRegistry.java:80: error: cannot find symbol
    public AtlasMeterRegistry(AtlasConfig config) {
                              ^
  symbol:   class AtlasConfig
  location: class AtlasMeterRegistry
/home/********/micrometer/implementations/micrometer-registry-atlas/src/main/java/io/micrometer/atlas/AtlasMeterRegistry.java:60: error: cannot find symbol
        this.registry = new AtlasRegistry(new com.netflix.spectator.api.Clock() {
                            ^
  symbol:   class AtlasRegistry
  location: class AtlasMeterRegistry
7 errors
3 warnings
brharrington commented 7 months ago

@rpalcolea @chali are either of you familiar with how the packaging gets setup for the POM when publishing only the shadow jar? I'm guessing this has to do with #1103 or #1104.

jonatan-ivanov commented 7 months ago

Hi,

Let me add some details to this. The project can be reproduced by Micrometer's build but I can also repro it with a minimal Gradle project:

apply plugin: 'java'

repositories {
    mavenCentral()
}

dependencies {
    implementation 'com.netflix.spectator:spectator-reg-atlas:1.7.6'
}

And you can try to use it in a simple Java class (e.g.: src/main/java/org/example/App.java):

package org.example;
public class App {
    public static void main(String[] args) {
        com.netflix.spectator.atlas.AtlasConfig atlasConfig = null;
    }
}

Which should result in the same compiler error as above:

package com.netflix.spectator.atlas does not exist

I used the latest Gradle (8.6, in Micrometer we are on 8.5), I also used Java 21 and MacOS:

JVM:          21 (BellSoft 21+37-LTS)
OS:           Mac OS X 14.3 x86_64

I don't think the java version or the OS is related since we have this issue on Micrometer CI too (Linux with different Java versions).

If I use Atlas 1.7.4, these are on my classpath:

com.netflix.spectator:spectator-api:1.7.4
com.netflix.spectator:spectator-ext-ipc:1.7.4
com.netflix.spectator:spectator-reg-atlas:1.7.4

If I use 1.7.6 or 1.7.5, spectator-reg-atlas disappears:

com.netflix.spectator:spectator-api:1.7.6
com.netflix.spectator:spectator-ext-ipc:1.7.6
rpalcolea commented 7 months ago

Hi folks,

I suspect there might be a regression in that fork. I opened https://github.com/Netflix/spectator/pull/1114 which:

brharrington commented 7 months ago

I suspect there might be a regression in that fork.

Thanks, I'll try out your patch. Since the fork is just the original project with recent changes added, it might be good to understand what the regression is. If the original starts getting updated again it seems likely it would pick up the same regression.

brharrington commented 7 months ago

@jonatan-ivanov 1.7.7 has been released and it works with micrometer when I tried locally.

jonatan-ivanov commented 7 months ago

Thank you! 1.7.7 passes on CI too I'm going to upgrade on Monday.