FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Configuration properties for Statsd's buffered and step properties are missing by izeye · Pull Request #30898 · spring-projects/spring-boot · GitHub

Configuration properties for Statsd's buffered and step properties are missing - #30898

Closed
izeye wants to merge 2 commits into
spring-projects:mainfrom
izeye:buffered
Closed

Configuration properties for Statsd's buffered and step properties are missing#30898
izeye wants to merge 2 commits into
spring-projects:mainfrom
izeye:buffered

Conversation

izeye commented May 8, 2022

Copy link
Copy Markdown
Contributor

This PR changes to expose buffered property for StatsdConfig.

See micrometer-metrics/micrometer#1375

wilkinsona commented May 8, 2022
edited
Loading

Copy link
Copy Markdown
Member

Thanks, @izeye. It's a shame that we missed this. I've opened #30901 to see if we can stop if from happening again.

There is a workaround for the missing property, but it's not very elegant:

@Bean
StatsdConfig statsdConfig(StatsdProperties statsdProperties) {
    return new StatsdPropertiesConfigAdapter(statsdProperties) {

        @Override
        public boolean buffered() {
            return false;
        }

    };
}

As such, I am a bit tempted to treat this as a bug and fix it in 2.5.x. Flagging for team attention to see what everyone else thinks.

wilkinsona added the for: team-attention An issue we'd like other members of the team to review label May 8, 2022

Copy link
Copy Markdown
Contributor

I think it's reasonable to consider this a bug of omission.

wilkinsona changed the title Expose buffered property for StatsdConfig Configuration property for Statsd's buffered property is missing May 12, 2022
wilkinsona added type: bug A general bug and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels May 12, 2022
wilkinsona added this to the 2.5.x milestone May 12, 2022

Copy link
Copy Markdown
Member

I've started looking at implementing something for #30901 and it looks like we're missing a configuration property for step as well. WDYT, @izeye? Does it make sense to have a configuration property for it?

wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label May 12, 2022

izeye commented May 12, 2022
edited
Loading

Copy link
Copy Markdown
Contributor Author

@wilkinsona I don't have much experience working with StatsD, but it seems to have been missed unintentionally, too and it sounds reasonable to align configurability level with its corresponding meter registry configuration.

/cc @shakuzen

spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels May 12, 2022

Copy link
Copy Markdown
Member

Statsd's step makes sense to expose as a configuration property and I suspect we just missed it.

Copy link
Copy Markdown
Member

Thanks, @shakuzen. @izeye, do you have time to update this PR to add step as well?

wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels May 16, 2022

izeye commented May 16, 2022

Copy link
Copy Markdown
Contributor Author

@wilkinsona Sure, I pushed ccbe758 for it.

spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels May 16, 2022
snicoll self-assigned this May 16, 2022
snicoll changed the title Configuration property for Statsd's buffered property is missing Configuration property for Statsd's buffered and step properties are missing May 16, 2022
snicoll closed this in e703acb May 16, 2022

snicoll commented May 16, 2022

Copy link
Copy Markdown
Member

Thanks again @izeye

wilkinsona changed the title Configuration property for Statsd's buffered and step properties are missing Configuration properties for Statsd's buffered and step properties are missing May 16, 2022
wilkinsona modified the milestones: 2.5.x, 2.5.14 May 16, 2022
izeye deleted the buffered branch May 16, 2022 15:01
wilkinsona removed the status: feedback-provided Feedback has been provided label May 16, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug A general bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL