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

Revisit MetricExportPropertiesConfiguration #5364

Closed
philwebb opened this issue Mar 8, 2016 · 5 comments
Closed

Revisit MetricExportPropertiesConfiguration #5364

philwebb opened this issue Mar 8, 2016 · 5 comments

Comments

@philwebb
Copy link
Member

philwebb commented Mar 8, 2016

The MetricExportProperties bean is a little odd, perhaps there's a better way to do it.

@philwebb philwebb self-assigned this Mar 8, 2016
@philwebb philwebb added this to the 1.4.0.M2 milestone Mar 8, 2016
@snicoll
Copy link
Member

snicoll commented Mar 9, 2016

That's also the only @ConfigurationProperites type with a Map to pojo as far as I know. If we could avoid that, that'd be great.

@dsyer
Copy link
Member

dsyer commented Apr 4, 2016

Using a Map of String to POJO is pretty common across all of Spring Cloud. If there's a better way I'd like to know what it is.

@philwebb philwebb modified the milestones: 1.4.0.M3, 1.4.0.M2 Apr 11, 2016
@philwebb philwebb modified the milestones: 1.4.0.RC1, 1.4.0.M3 May 17, 2016
@philwebb
Copy link
Member Author

I was specifically referring to this code:

    @Configuration
    protected static class MetricExportPropertiesConfiguration {

        @Value("${spring.application.name:application}.${random.value:0000}")
        private String prefix = "";

        private String aggregateKeyPattern = "k.d";

        @Bean(name = "spring.metrics.export-org.springframework.boot.actuate.metrics.export.MetricExportProperties")
        @ConditionalOnMissingBean
        public MetricExportProperties metricExportProperties() {
            MetricExportProperties export = new MetricExportProperties();
            export.getRedis().setPrefix("spring.metrics"
                    + (this.prefix.length() > 0 ? "." : "") + this.prefix);
            export.getAggregate().setPrefix(this.prefix);
            export.getAggregate().setKeyPattern(this.aggregateKeyPattern);
            return export;
        }

    }

@dsyer
Copy link
Member

dsyer commented May 25, 2016

I suppose you could put some of that in a @PostConstruct. The reason it's like that, though, is so there's an escape hatch, and the user can provide their own defaults if needed.

@wilkinsona wilkinsona removed this from the 1.4.0.RC1 milestone Jul 5, 2016
@wilkinsona
Copy link
Member

This has been superseded by the planned move to Micrometer-based metrics (#9970)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants