Fix for #81: For the Prometheus exporter, the values based on ElecticityMaps provider are estimations and not the real values #83

locomundo · 2024-06-07T13:05:34Z

This is the fix discussed in issue #81.

We noticed that when using the grid-intensity exporter with ElectricityMaps as provider we always obtained an estimation, never the real value.

By using the history endpoint as opposed to the latest one, we can get both the latest estimated value, as well as the latest real measured value.

This change includes:

For ElectricityMaps, we now use the history endpoint instead of the latest one, to return both the latest estimated and latest real values.
The exporter now sends the timestamp with the Prometheus metrics, as the latest real value is usually in the past in the case of ElectricityMaps.
Tested that it works with ElectricityMaps, Ember and CarbonIntensityOrgUK, and Watttime providers (so all of them), but didn't run the tests.
The label is_estimated has been added in the Prometheus metrics. For ElectricityMaps it can provide values for estimated or real, or for one of the two, depending on the location. For example, for NL we get:

grid_intensity_carbon_average{is_estimated="false",location="NL",node="",provider="ElectricityMaps",region="",units="gCO2e per kWh"} 92 1717758000000
grid_intensity_carbon_average{is_estimated="true",location="NL",node="",provider="ElectricityMaps",region="",units="gCO2e per kWh"} 94 1717765200000

For the other providers it always returns the value as is_estimated=false (the default). Is this correct or are you aware of some of the other providers returning estimated data? @rossf7 @mrchrisadams

thegreenwebfoundation#81 - Now sending always the timestamp in the Prometheus metrics. - For ElectricityMaps, we now use the historic endpoint instead of the carbon-intensity one, so we can get both the latest estimated and real values. - Tested that it works with Ember and CarbonIntensityOrgUK, Watttime not yet (I need to register) - The label is_estimated has been added in the Prometheus metrics. For ElectricityMaps it can provide values for both, or for one of the two, depending on the location. - For the other providers it always returns the value as is_estimated=false (the default) because I didn't make any changes. I'm. not 100% sure this is correct for all of them, or if some of the providers also return estimations (to investigate). - The error handling when parsing generates a lot of clutter, not sure if there's a better way to do it in go...

for relative carbon intensity Otherwise we see this error, for example: panic: inconsistent label cardinality: expected 5 label values but got 6 in []string{"CAISO_NORTH", "", "WattTime", "", "percent", "false"}

mrchrisadams · 2024-06-10T10:58:16Z

Thanks for this @locomundo, this looks great!

I'm currently travelling but if @rossf7 doesn't get to it before Wednesday I'll aim to review it by the end of the week.

rossf7 · 2024-06-10T12:39:54Z

@locomundo Yes this looks good. I think / hope we can get it merged this week and then cut a release with it and #82

For the other providers it always returns the value as is_estimated=false (the default). Is this correct or are you aware of some of the other providers returning estimated data?

For the other providers I think we'll need to check their methodologies. Unless you know @mrchrisadams ?

but if @rossf7 doesn't get to it before Wednesday I'll aim to review it by the end of the week.

I should be able to review first half of this week. Chris, I'll let you know if I get delayed

mrchrisadams · 2024-06-11T07:08:22Z

Hey @rossf7 - I think pretty much all of these are forecasts / estimates rather than historical, recorded figures.

Ember - the free annual figures we use are an estimate, based on either the latest data for the last full year, or data from the last full month. In each case, if we used the Ember data to get an idea of the carbon intensity for "now", I think we'd be looking at historical data as the basis to project forward to now. More in the methodology documentation here

CarbonIntensityOrgUK - the figures here are forecasts, based on an ML model. I don't think it's historical data per se. They briefly describe the M methodology on their front page

Watttime - I'm less clear on Watttime. Because marginal emissions rely on a comparison to a counterfactual that doesn't exist (we don't have two versions of the same grid to compare side by side), I think you'd refer to all of these as estimates, until there was some explicit validation for each reading. I think this is somewhat comparable to Electricity Maps saying a reading is estimated until it is replaced with 'real' data. You can read some more about the Watttime methodologies here

Until we hear otherwise, I think it's safer to assume all the other readings are estimates.

locomundo · 2024-06-11T07:28:53Z

In that case we can invert it and make the default as isEstimated=true. If you all agree on this I can make that change in the PR @rossf7 @mrchrisadams

rossf7 · 2024-06-11T08:21:43Z

@locomundo Yes, good idea lets invert it. @mrchrisadams thanks for clarifying and I agree for WattTime since they return marginal emissions we should treat them as estimated.

Thus, we are setting IsEstimated to true. See for more info thegreenwebfoundation#83 (comment)

locomundo · 2024-06-11T09:17:02Z

Ok, so now all the other providers are returning is_estimated=true. No default, though, I thought I could change the default value, but that was not possible. However, this might be better so it's explicit and clear.

rossf7

@locomundo This is really cool to be able to show both values!

Just some minor nits from me and then I think this is good to go.

rossf7 · 2024-06-11T10:04:29Z

pkg/provider/electricity_maps.go

@@ -64,34 +64,139 @@ func (e *ElectricityMapsClient) GetCarbonIntensity(ctx context.Context, location
 		return nil, errBadStatus(resp)
 	}

-	data := &electricityMapsData{}
-	err = json.NewDecoder(resp.Body).Decode(data)
+	type EMHistoryResponse struct {


Suggested change

type EMHistoryResponse struct {

type electricityMapsHistoryResponse struct {

Rather than have this struct inline could you rename it and add it to the end of the file?

For now we have all the providers in a single package but we use the provider name as a prefix for the types.

rossf7 · 2024-06-11T10:09:55Z

pkg/provider/electricity_maps.go

+
+// Helper struct to remove clutter in the calling function
+// while finding the latest (and greatest) data points
+type mostRecentDatapoints struct {


Suggested change

type mostRecentDatapoints struct {

type electricityMapsDatapoints struct {

Same here to add the prefix.

pkg/provider/electricity_maps.go

Co-authored-by: Ross Fairbanks <[email protected]>

rossf7

LGTM 🚀

in Prometheus. Related to PR thegreenwebfoundation#83 "For the Prometheus exporter, the values based on ElecticityMaps provider are estimations and not the real values"

* Fix/improvement for issue #81 #81 - Now sending always the timestamp in the Prometheus metrics. - For ElectricityMaps, we now use the historic endpoint instead of the carbon-intensity one, so we can get both the latest estimated and real values. - Tested that it works with Ember and CarbonIntensityOrgUK, Watttime not yet (I need to register) - The label is_estimated has been added in the Prometheus metrics. For ElectricityMaps it can provide values for both, or for one of the two, depending on the location. - For the other providers it always returns the value as is_estimated=false (the default) because I didn't make any changes. I'm. not 100% sure this is correct for all of them, or if some of the providers also return estimations (to investigate). - The error handling when parsing generates a lot of clutter, not sure if there's a better way to do it in go... * Fix for Watttime - was missing the is_estimated label... for relative carbon intensity Otherwise we see this error, for example: panic: inconsistent label cardinality: expected 5 label values but got 6 in []string{"CAISO_NORTH", "", "WattTime", "", "percent", "false"} * Cleaned up some commented code * It turns out that all the other providers return estimated values Thus, we are setting IsEstimated to true. See for more info #83 (comment) * Some renaming as suggested by Chris * Update pkg/provider/electricity_maps.go Co-authored-by: Ross Fairbanks <[email protected]> * Document how to fix "old" samples not being ingested in Prometheus. Related to PR #83 "For the Prometheus exporter, the values based on ElecticityMaps provider are estimations and not the real values" * Corrections suggested by Ross Co-authored-by: Ross Fairbanks <[email protected]> --------- Co-authored-by: Flavia Paganelli <[email protected]> Co-authored-by: Ross Fairbanks <[email protected]>

Flavia Paganelli and others added 4 commits June 3, 2024 17:24

Fix for Watttime - was missing the is_estimated label...

0904e0c

for relative carbon intensity Otherwise we see this error, for example: panic: inconsistent label cardinality: expected 5 label values but got 6 in []string{"CAISO_NORTH", "", "WattTime", "", "percent", "false"}

Cleaned up some commented code

7c0adf2

Merge branch 'main' into main

1769981

Flavia Paganelli added 2 commits June 11, 2024 11:14

It turns out that all the other providers return estimated values

6060179

Thus, we are setting IsEstimated to true. See for more info thegreenwebfoundation#83 (comment)

Merge branch 'main' of github.com:aknostic/grid-intensity-go

0b3e9d5

rossf7 reviewed Jun 11, 2024

View reviewed changes

Flavia Paganelli and others added 2 commits June 11, 2024 13:40

Some renaming as suggested by Chris

e5ffba1

Update pkg/provider/electricity_maps.go

55f4695

Co-authored-by: Ross Fairbanks <[email protected]>

rossf7 approved these changes Jun 11, 2024

View reviewed changes

rossf7 merged commit cc6e43d into thegreenwebfoundation:main Jun 11, 2024

This was referenced Aug 23, 2024

Prometheus error ingesting samples too old using electricity maps #86

Closed

Document how to enable ingest old data #87

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for #81: For the Prometheus exporter, the values based on ElecticityMaps provider are estimations and not the real values #83

Fix for #81: For the Prometheus exporter, the values based on ElecticityMaps provider are estimations and not the real values #83

locomundo commented Jun 7, 2024

mrchrisadams commented Jun 10, 2024

rossf7 commented Jun 10, 2024

mrchrisadams commented Jun 11, 2024 •

edited

Loading

locomundo commented Jun 11, 2024

rossf7 commented Jun 11, 2024

locomundo commented Jun 11, 2024

rossf7 left a comment

rossf7 Jun 11, 2024

locomundo Jun 11, 2024

rossf7 Jun 11, 2024

locomundo Jun 11, 2024

rossf7 left a comment

	type EMHistoryResponse struct {
	type electricityMapsHistoryResponse struct {

	type mostRecentDatapoints struct {
	type electricityMapsDatapoints struct {

Fix for #81: For the Prometheus exporter, the values based on ElecticityMaps provider are estimations and not the real values #83

Fix for #81: For the Prometheus exporter, the values based on ElecticityMaps provider are estimations and not the real values #83

Conversation

locomundo commented Jun 7, 2024

mrchrisadams commented Jun 10, 2024

rossf7 commented Jun 10, 2024

mrchrisadams commented Jun 11, 2024 • edited Loading

locomundo commented Jun 11, 2024

rossf7 commented Jun 11, 2024

locomundo commented Jun 11, 2024

rossf7 left a comment

Choose a reason for hiding this comment

rossf7 Jun 11, 2024

Choose a reason for hiding this comment

locomundo Jun 11, 2024

Choose a reason for hiding this comment

rossf7 Jun 11, 2024

Choose a reason for hiding this comment

locomundo Jun 11, 2024

Choose a reason for hiding this comment

rossf7 left a comment

Choose a reason for hiding this comment

mrchrisadams commented Jun 11, 2024 •

edited

Loading