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

Add project IDs to error logs #362

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

TylerLubeck
Copy link
Contributor

When exporting metrics from multiple projects at once, it's useful to know which of the projects is generating metrics errors.

Heya @SuperQ , tagging you per the contributors guide. Mind taking a look when you get a minute?

@SuperQ SuperQ requested a review from kgeckhart August 27, 2024 16:53
@@ -416,7 +416,7 @@ func (c *MonitoringCollector) reportTimeSeriesMetrics(
c.aggregateDeltas,
)
if err != nil {
return fmt.Errorf("error creating the TimeSeriesMetrics %v", err)
return fmt.Errorf("error creating the TimeSeriesMetrics for project %s %v", c.projectID, err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will duplicate the data since we add it in the error logger above.

@SuperQ
Copy link
Contributor

SuperQ commented Aug 29, 2024

This needs a DCO sign-off. You can use git commit -s --amend to add it.

Copy link
Contributor

@kgeckhart kgeckhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

A small optional enhancement I can see would be to add a logger = log.With(logger, "project_id", c.projectID) in the constructor https://github.com/prometheus-community/stackdriver_exporter/blob/master/collectors/monitoring_collector.go#L123-L126 that way every log in the collector has the project_id attached.

When exporting metrics from multiple projects at once, it's useful to
know which of the projects is generating metrics errors.

Signed-off-by: Tyler Lubeck <[email protected]>
@TylerLubeck
Copy link
Contributor Author

Updated both - sorry for the delay y'all, I appear to have broken my GitHub notifications

@SuperQ
Copy link
Contributor

SuperQ commented Sep 5, 2024

Nice

@SuperQ SuperQ merged commit d8b48ad into prometheus-community:master Sep 5, 2024
4 checks passed
@kgeckhart kgeckhart mentioned this pull request Oct 7, 2024
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

Successfully merging this pull request may close these issues.

3 participants