Skip to content
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.

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

InfluxDB 2.0 API support #1974

Closed
AarjavP opened this issue Apr 7, 2020 · 24 comments · Fixed by #2113
Closed

InfluxDB 2.0 API support #1974

AarjavP opened this issue Apr 7, 2020 · 24 comments · Fixed by #2113
Labels
enhancement A general enhancement registry: influx An InfluxDB Registry related issue spring-boot change Change is needed in Spring Boot for this issue
Milestone

Comments

@AarjavP
Copy link

AarjavP commented Apr 7, 2020

Looks like the 'direct' api for 1.x will not work with 2.0.
Thankfully the changes seem pretty small, for comparison 1.x write api and 2.x write api

The body remains the same (line protocol), only major change I believe is for auth (though user + password is probably also possible), instead of db query param there is org and bucket.
There is also an official java client library, though I am not sure if micrometer needs all the functionality it provides.

Sorry if there is already something in place to configure micrometer so it works with influxdb 2.0 (directly), but I did not find it.

@AarjavP
Copy link
Author

AarjavP commented Apr 7, 2020

Looks like this is already being worked on: #1423

@shakuzen
Copy link
Member

shakuzen commented Apr 8, 2020

It looks like 2.0 is still beta according to the docs. That doesn't mean we can't support it, necessarily, but we don't want to put something out there that claims to be stable when the integration isn't a final version yet.

@shakuzen shakuzen added the registry: influx An InfluxDB Registry related issue label Apr 8, 2020
@AarjavP
Copy link
Author

AarjavP commented Apr 8, 2020

Yes, that makes sense. In the meantime, I can help with testing or anything else that may be needed to prepare.

@bednar
Copy link
Contributor

bednar commented May 7, 2020

Hi @shakuzen,

The PR #1423 is rebased and ready to review. This PR introduce a new registry influx2, but I could create a new PR that will improve existing influx registry with support to InfluxDB 2.

What is a better approach?

Regards

@jkschneider
Copy link
Contributor

@bednar Without having a look at all the code, can we make the distinction between Influx 1 and 2 fully transparent to the user, i.e. can Micrometer somehow check for v2 support via some API endpoint and just use it and fall back on v1 otherwise?

Supposing this is not possible, we could introduce an InfluxConfig option for Influx version (carefully validated to an acceptable range of values).

Only if the other configuration options differ substantially between Influx 1 and 2 would I consider a separate MeterRegistry implementation. In other words, I want to avoid two disjoint sets of config options in one InfluxConfig interface where it isn't clear to the user what they need to configure for which version.

@bednar
Copy link
Contributor

bednar commented May 11, 2020

Hi @jkschneider,

We could do distinction via configuration. If user sets the db then we use v1 API, If user sets the bucket then we use v2 API.

Micrometer required parameters:

v1 v2
uri uri
db bucket
consistency org
token

and validation of InfluxConfig will be look something like:

@Override
default Validated<?> validate() {
    return checkAll(this,
            c -> StepRegistryConfig.validate(c),
            checkRequired("uri", Influx2Config::uri),
            checkOneOf(
                    checkRequired("db", Influx2Config::bucket)
                            .andThen(checkRequired("consistency", Influx2Config::token))),
                    checkRequired("bucket", Influx2Config::bucket)
                            .andThen(checkRequired("org", Influx2Config::org))
                            .andThen(checkRequired("token", Influx2Config::token))),
    );
}

Documentation could looks like:

Screenshot 2020-05-11 at 08 50 29
Screenshot 2020-05-11 at 08 45 14

What do you think? It is an acceptable approach?

Regards

@jkschneider
Copy link
Contributor

@bednar It seems that conceptually bucket is similar to db. We'll come back to that in a moment...

  • I think only Influx 1.x has a /ping API that returns the version. If this returns HTTP 200, assume 1.x. Can we HEAD this instead of GET?
  • Influx 1.8.0 has a forward-compatible API /api/v2/write and /api/v2/query. If present, write v2 format to these endpoints.
  • Else write v1 to /api/write.

Returning then to bucket and db... They both basically configure "where metrics are being stored" roughly. I'd suggest we simply add one as an alias to the other, so the user can use whatever storage concept is familiar to them based on major version. As 1.x becomes less prevalent (if it hasn't already), we can deprecate db in favor of bucket.

@bednar
Copy link
Contributor

bednar commented May 12, 2020

@jkschneider thanks for response.

We could use db as an alias to the bucket, but what about org and token? These parameters we have to pass to HTTP request during writes into Influx v2.

If we want to store metrics into v2, this configuration is invalid:

management.metrics.export.influx:
    db: mydb
    uri: http://localhost:9999

Will we use a default values for org and token parameters?

  • I think only Influx 1.x has a /ping API that returns the version. If this returns HTTP 200, assume 1.x. Can we HEAD this instead of GET?

Yes, we can use HEAD instead GET.

@jkschneider
Copy link
Contributor

but what about org

Influx 2 requires you to establish an org on influx setup, but there is no default name. I'd propose that we make org nullable and when null and using Influx 2, try to retrieve the org from /api/v2/orgs. If there is more than one org listed there, fail validation.

but what about... token

Influx 1.x supported JWT tokens (i.e. Authorization: Bearer header in 1.x and Authorization: Token header in 2.x), so we should probably support token-based authentication in both anyway.

@bednar
Copy link
Contributor

bednar commented May 13, 2020

@jkschneider thanks for explanation. One more question...

If there is more than one org listed there, fail validation.

Where will do this validation? In the InfluxConfig.validate() or during a first publish() in the InfluxMeterRegistry?

@jkschneider
Copy link
Contributor

Where will do this validation?

Good question, and I thought about this a bit too. I think it would have to be in the first publish() because the user might inject a custom HttpSender.

Which brings up another point... We should probably introduce a config to allow for the user to manually select API version as a fallback in case they do something like inject an HttpSender that shovels metrics over Kafka, in which case a HEAD to /ping is going to fail regardless. I know this is getting complicated. Perhaps forget this last point for now and we can iron out the details at the end.

@bednar
Copy link
Contributor

bednar commented May 14, 2020

Where will do this validation?

Good question, and I thought about this a bit too. I think it would have to be in the first publish() because the user might inject a custom HttpSender.

Ok, it sounds good.

Which brings up another point... We should probably introduce a config to allow for the user to manually select API version as a fallback in case they do something like inject an HttpSender that shovels metrics over Kafka, in which case a HEAD to /ping is going to fail regardless. I know this is getting complicated. Perhaps forget this last point for now and we can iron out the details at the end.

Ok.

I will prepare a second PR that will improve existing InfluxMeterRegistry.java .

@AarjavP
Copy link
Author

AarjavP commented May 14, 2020

Sorry I am late to respond to this thread but would it be possible to keep the two different registries, and have an additional delegating one which does the 'detection' of which version to use? (Though it makes perfect sense for them both to share the code regarding line protocol like in the PR) In my deployments I will know the version ahead of time and would just like to the v2 directly, and fail if there is an issue.

Then again I am still pretty inexperienced so I will trust your judgement but just wanted to voice my thoughts once.

@bednar
Copy link
Contributor

bednar commented May 20, 2020

I've prepare a draft PR #2113 which introduce a support of InfluxDB v2 in the influx registry.

It is not finished but I think it is a good start point to discuss.

@bednar
Copy link
Contributor

bednar commented May 21, 2020

#2113 is ready to review

@DangerousDarlow
Copy link

It would be great to have this support in Micrometer. I am using InfluxDB Cloud as I'm working on a pet project and don't want the cost or hassle of hosting my own Influx DB instance.

@bednar
Copy link
Contributor

bednar commented May 26, 2020

cc @AarjavP @DangerousDarlow

I've prepared an example how to use a current version of Spring Boot to sent metrics into the InfluxDB v2.

We had to use a custom HttpSender to add required parameters into a HTTP request:

Custom HttpSender

package io.micrometer.boot2.samples;

import io.micrometer.boot2.samples.components.PersonController;
import io.micrometer.core.instrument.Clock;
import io.micrometer.core.ipc.http.OkHttpSender;
import io.micrometer.influx.InfluxConfig;
import io.micrometer.influx.InfluxMeterRegistry;
import okhttp3.HttpUrl;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.builder.SpringApplicationBuilder;
import org.springframework.context.annotation.Bean;
import org.springframework.scheduling.annotation.EnableScheduling;

@SpringBootApplication(scanBasePackageClasses = PersonController.class)
@EnableScheduling
public class InfluxSample {
    public static void main(String[] args) {
        new SpringApplicationBuilder(InfluxSample.class).profiles("influx").run(args);
    }

    @Value("${management.metrics.export.influx.org}")
    private String org;

    @Value("${management.metrics.export.influx.bucket}")
    private String bucket;

    @Value("${management.metrics.export.influx.token}")
    private String token;

    @Bean
    public InfluxMeterRegistry influxMeterRegistry(InfluxConfig influxConfig, Clock clock) {

        OkHttpClient.Builder httpClient = new OkHttpClient.Builder();

        httpClient.addInterceptor(chain -> {
            Request original = chain.request();

            // skip others than write
            if (!original.url().pathSegments().contains("write")) {
                 return  chain.proceed(original);
            }

            HttpUrl url = original.url()
                    .newBuilder()
                    .removePathSegment(0)
                    .addEncodedPathSegments("api/v2/write")
                    .removeAllQueryParameters("db")
                    .removeAllQueryParameters("consistency")
                    .addQueryParameter("org", org)
                    .addQueryParameter("bucket", bucket)
                    .build();

            Request request = original.newBuilder()
                    .url(url)
                    .header("Authorization", "Token " + token)
                    .build();

            return chain.proceed(request);
        });

        return InfluxMeterRegistry.builder(influxConfig)
                .clock(clock)
                .httpClient(new OkHttpSender(httpClient.build()))
                .build();
    }
}

Application configuration

management.metrics.export:
  influx:
    enabled: true
    step: PT10S
    readTimeout: PT30S
    batchSize: 20000
    uri: "http://localhost:9999"
    org: "my-org"
    bucket: "my-bucket"
    token: "my-token"

@AarjavP
Copy link
Author

AarjavP commented Nov 11, 2020

Seems InfluxDB v2 has been released. https://docs.influxdata.com/influxdb/v2.0/reference/release-notes/influxdb/ 🎉

@coditori
Copy link

coditori commented Dec 6, 2020

Hi guys, is there any update? I wonder why this issue is closed :|

@shakuzen shakuzen added this to the 1.7.0 milestone Mar 2, 2021
@shakuzen shakuzen added the enhancement A general enhancement label Mar 2, 2021
@shakuzen
Copy link
Member

shakuzen commented Mar 2, 2021

is there any update? I wonder why this issue is closed :|

The issue is not closed. I've just added it to the 1.7 milestone, which is our upcoming feature release.

@shakuzen
Copy link
Member

This is available in the latest 1.7.0-SNAPSHOT builds. Please feel free to try it out and leave any feedback. We'll have a 1.7 milestone release out soon.

@vadim-hleif
Copy link

Hi guys, are there any examples of configs?

management:
  metrics:
    export:
      influx:
        enabled: true
        step: PT0.5S
        readTimeout: PT30S
        batchSize: 20000
        uri: http://localhost:8086
        org: org
        bucket: test-bucket
        token: token

It doesn't work for me:

2021-03-16 10:31:16,169Z ERROR [SpringContextShutdownHook] io.micrometer.influx.InfluxMeterRegistry: unable to create database 'mydb': {"code":"unauthorized","message":"unauthorized access"}
2021-03-16 10:31:16,185Z ERROR [SpringContextShutdownHook] io.micrometer.influx.InfluxMeterRegistry: failed to send metrics to influx: {"code":"unauthorized","message":"unauthorized access"}

It doesn't work when i change bucket to db and token to password.
But after changing bucket -> db i can see test-bucket instead of mydb in the error.

deps:

implementation("io.micrometer:micrometer-core:1.7.0-SNAPSHOT")
implementation("io.micrometer:micrometer-registry-influx:1.7.0-SNAPSHOT")

@shakuzen
Copy link
Member

@vadim-hleif you will have to make an InfluxConfig bean with the configuration you want until we update the configuration properties support in Spring Boot to know about the new properties.

@Bean
InfluxConfig influxConfig() {
    return new InfluxConfig() {
        @Override
        public String get(String key) {
            return null;
        }

        @Override
        public Duration step() {
            return Duration.ofSeconds(20);
        }

        @Override
        public String org() {
            return "org";
        }

        @Override
        public String bucket() {
            return "metrics";
        }

        @Override
        public String token() {
            return "my-token";
        }
    };
}

@shakuzen shakuzen added the spring-boot change Change is needed in Spring Boot for this issue label Mar 16, 2021
@shakuzen
Copy link
Member

shakuzen commented Mar 23, 2021

This is included in Micrometer 1.7.0-M1 and Spring Boot support has been updated to handle the new properties in Spring Boot 2.5.0-M3. Please try it out and let us know any feedback on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A general enhancement registry: influx An InfluxDB Registry related issue spring-boot change Change is needed in Spring Boot for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants