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

Log CloudWatch metric data when publishing fails #1396

Closed
danieljamesscott opened this issue Apr 24, 2019 · 10 comments · Fixed by #4773
Closed

Log CloudWatch metric data when publishing fails #1396

danieljamesscott opened this issue Apr 24, 2019 · 10 comments · Fixed by #4773
Labels
enhancement A general enhancement registry: cloudwatch A CloudWatch Registry related issue
Milestone

Comments

@danieljamesscott
Copy link

When a failure occurs while sending to cloudwatch, I can see the exception in the logs:

https://github.com/micrometer-metrics/micrometer/blob/master/implementations/micrometer-registry-cloudwatch/src/main/java/io/micrometer/cloudwatch/CloudWatchMeterRegistry.java#L116

It would be useful to log the metricData, or possibly putMetricDataRequest. Would you accept a PR which does this?

@izeye
Copy link
Contributor

izeye commented Apr 24, 2019

@danieljamesscott Thanks for the suggestion! Sounds reasonable to me. FYI we already have error request logging for Elasticsearch. See https://github.com/micrometer-metrics/micrometer/blob/master/implementations/micrometer-registry-elastic/src/main/java/io/micrometer/elastic/ElasticMeterRegistry.java#L171

@danieljamesscott
Copy link
Author

So would you want to only log it on debug? I would think that it would be OK to log it in the error log line?

@izeye
Copy link
Contributor

izeye commented Apr 24, 2019

@danieljamesscott I think it should be debug for consistency. If you think error fits better, you'd better open a separate issue for it as the change should be done across all meter registries consistently.

@danieljamesscott
Copy link
Author

My PR logs request/response data when there is an error sending to the registries

@izeye
Copy link
Contributor

izeye commented Apr 24, 2019

@danieljamesscott Thanks for creating the PR but I expected to create a separate issue for discussion if you want to change the existing log level. I don't have a strong opinion on it but looking at #1289 (comment), @shakuzen seems to have an opinion on this, so I thought it'd be better to discuss it in a dedicated issue before acting on it.

@danieljamesscott
Copy link
Author

Yep, understood. But logging on debug is useless for me because it's inconsistently reproducible - I can't run with debug logging enabled all the time. I'm using an extended CloudWatchMeterRegistry containing the required logging - so the problem is solved for me.

@shakuzen
Copy link
Member

But logging on debug is useless for me because it's inconsistently reproducible - I can't run with debug logging enabled all the time.

You should be able to configure your logging system to log DEBUG level for a specific logger (often corresponding to a single class), rather than all logging. Would that work for your purposes? Configuring the logging system as needed seems preferable to a custom MeterRegistry implementation.

@marcingrzejszczak marcingrzejszczak added the waiting for feedback We need additional information before we can continue label Dec 19, 2023
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

Copy link

github-actions bot commented Jan 7, 2024

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 7, 2024
@jonatan-ivanov jonatan-ivanov removed waiting for feedback We need additional information before we can continue feedback-reminder labels Jan 8, 2024
@izeye
Copy link
Contributor

izeye commented Feb 28, 2024

I created #4773 to try to resolve this.

@jonatan-ivanov jonatan-ivanov added enhancement A general enhancement and removed closed-as-inactive labels Feb 28, 2024
@jonatan-ivanov jonatan-ivanov added this to the 1.13.0-M2 milestone Feb 28, 2024
@jonatan-ivanov jonatan-ivanov changed the title Log cloudwatch metric data when unable to send to cloudwatch Log CloudWatch metric data when publishing fails Feb 29, 2024
@jonatan-ivanov jonatan-ivanov added the registry: cloudwatch A CloudWatch Registry related issue label Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A general enhancement registry: cloudwatch A CloudWatch Registry related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants