Wavefront Proxy validation error with default uri implementation

[rrourke]

Using the Wavefront Proxy I keep getting a uri error

  <dependency>
      <groupId>io.micrometer</groupId>
      <artifactId>micrometer-registry-wavefront</artifactId>
      <version>1.10.6</version>
  </dependency>

! io.micrometer.core.instrument.config.validate.ValidationException: wavefront.uri was 'proxy://xxx.yyy.com:2878' but it must be a valid URL
! at io.micrometer.core.instrument.config.validate.Validated$Invalid.get(Validated.java:304)

No matter how I try set up the url it reports its incorrect even though the uri is correct.

[shakuzen]

I tried to reproduce the issue you mentioned in a unit test, but it passes without issue. Could you please provide a unit test demonstrating how to reproduce the error you are seeing?

@Test
void proxyConfigIsValid() {
    WavefrontConfig config = new WavefrontConfig() {
        @Override
        public String get(String key) {
            return null;
        }

        @Override
        public String uri() {
            return "proxy://example.com:2878";
        }
    };
    assertThat(config.validate().isValid()).isTrue();
}

[shakuzen]

I also tried one without overriding the default uri method using a properties file as follows:

@Test
void proxyConfigIsValid() throws IOException {
    Properties wavefrontProps = new Properties();
    wavefrontProps.load(this.getClass().getResourceAsStream("/wavefront.properties"));
    WavefrontConfig config = wavefrontProps::getProperty;
    assertThat(config.validate().isValid()).isTrue();
}

With the wavefront.properties containing:

wavefront.uri=proxy://example.com:2878

[mrniko]

@shakuzen

Can you try the config below?

        WavefrontConfig config = new WavefrontConfig() {

            @Override
            public String get(String key) {
                if (key.endsWith(".uri")) {
                    return "proxy://example.com:2878";
                }
                return null;
            }
        };

[jonatan-ivanov]

@mrniko Did you try it? To me it is still valid (but I would not recommend doing this since it's a bit hacky.)

@rrourke Were you able to figure out what was wrong? Could you please share, it might be useful to others.

[rrourke]

Hi @jonatan-ivanov @mrniko is the author of the sdk I was using. Would have to ask him but I think maybe was using an older version in his sdk but will see what he says.

[mrniko]

@jonatan-ivanov

Here is the WavefrontConfig which works for all parameters except uri.

        WavefrontConfig config = new WavefrontConfig() {

            @Override
            public String uri() {
                return uri;
            }

            @Override
            public String get(String key) {
                if (key.endsWith(".step")) {
                    return step;
                }
                if (key.endsWith(".batchSize")) {
                    return batchSize;
                }
                if (key.endsWith(".numThreads")) {
                    return numThreads;
                }
                if (key.endsWith(".uri")) {
                    return uri;
                }
                if (key.endsWith(".source")) {
                    return source;
                }
                if (key.endsWith(".apiToken")) {
                    return apiToken;
                }
                return null;
            }
        };

Does your answer mean that it's better to always return null in public String get(String key) method implementation?

[jonatan-ivanov]

Here is the WavefrontConfig which works for all parameters except uri.

I see two issues with your example:

1. It does not compile
2. You have uri implemented and key.endsWith(".uri") in get

If I fix the compilation issues and substitute them with real values, it works for me, I don't get errors.

Does your answer mean that it's better to always return null in public String get(String key) method implementation?

Did you see what @shakuzen did above in #3903 (comment)?
You can do the same with a Map and you can implement the different methods and return null in get.

[shakuzen]

Does your answer mean that it's better to always return null in public String get(String key) method implementation?

The get method is used by the default implementations of the other methods. If you are overriding the implementations of the other methods, then your implementation of that method will be used instead. This arrangement enables you to back the config values with an arbitrary source, like a Properties object (which is backed by e.g. a .properties file) or a Map or some external configuration by retrieving values from those in the implementation of the get method.

In your example, it looks like you already have everything in fields. You could override the implementation of each method to directly return the respective field. If you do that, there's no benefit to doing anything in the get implementation, because you'll have overridden all the other methods so get isn't called anymore.

As Jonatan mentioned, the example you provided passes the test. In the second example, it would depend what the value of uri is, but if it is the same value as in other examples, it would work also.

[mrniko]

@shakuzen

Sorry, my mistake.

The config provided in this example won't work - #3903 (comment) if uri() method isn't overriden. Can you check that?

[shakuzen]

The config provided in this example won't work - #3903 (comment) if uri() method isn't overriden. Can you check that?

I did try it, and the following test passes. Does that not pass for you? What are we missing?

@Test
void proxyConfigIsValid() {
    WavefrontConfig config = new WavefrontConfig() {

        @Override
        public String get(String key) {
            if (key.endsWith(".uri")) {
                return "proxy://example.com:2878";
            }
            return null;
        }
    };
    assertThat(config.validate().isValid()).isTrue();
}

[mrniko]

Thank you for the provided test. Unfortunately, the test below will fail.

        WavefrontConfig config = new WavefrontConfig() {
            @Override
            public String get(String key) {
                if (key.endsWith(".uri")) {
                    return "proxy://example.com:2878";
                }
                return null;
            }
        };

        WavefrontMeterRegistry r = new WavefrontMeterRegistry(config, Clock.SYSTEM);

[shakuzen]

Thanks for the complete test that reproduces it. Indeed that does not work due to the validation we're doing in the default uri method. We can consider relaxing that validation, but in the meantime, you will indeed need to override the uri method to avoid the issue with a Wavefront proxy config.

[mrniko]

@shakuzen

Thank you!