Using new otlpfile pipeline for self metrics #1922
Conversation
| // Wait until the otlpjsonfile receiver writes the metrics with the | ||
| // correct labels. | ||
| time.Sleep(2 * time.Minute) | ||
| testDefaultMetrics(ctx, t, logger, vm, time.Minute) |
There was a problem hiding this comment.
Two questions :
- Why not sleeping ~1min instead ?
- Why do we update this window parameter to
time.Minute? I think this would make the query more restrictive to find default metric datapoints and cause the test to be slower and potentially flaky.
confgenerator/confgenerator.go
Outdated
| @@ -100,6 +100,12 @@ func (uc *UnifiedConfig) GenerateOtelConfig(ctx context.Context) (string, error) | |||
| ReceiverPipelineName: "otel", | |||
| } | |||
|
|
|||
| receiverPipelines["enabled_receivers_feature_tracking"] = EnabledReceiversFeatureTrackingMetricsPipeline(ctx, outDir) | |||
There was a problem hiding this comment.
I suggest a more descriptive name for the pipeline instead of enabled_receivers_feature_tracking. A suggestion is otlp_agent_self_metrics.
Another alternative is to call the pipeline ops_agent since these metrics are Ops Agent self metrics. This following the naming of the others otel and fluentbit which represent self metrics about subagents.
| ) | ||
|
|
||
| var ( | ||
| config = flag.String("config", "/etc/google-cloud-ops-agent/config.yaml", "path to the user specified agent config") | ||
| ) | ||
|
|
||
| func run(ctx context.Context) error { | ||
| userUc, mergedUc, err := utils.GetUserAndMergedConfigs(ctx, *config) | ||
| _, _, err := utils.GetUserAndMergedConfigs(ctx, *config) |
There was a problem hiding this comment.
This method can just return an err now. We don't ever use the first 2 return values I don't think
There was a problem hiding this comment.
This all will go away on the next PR that removes the diagnostics binary. I'm trying to change the least i can and maintain functionality to make it easier to review.
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| ctx, cancel := context.WithCancel(ctx) | ||
| _, cancel := context.WithCancel(ctx) |
There was a problem hiding this comment.
Do we need this anymore then?
There was a problem hiding this comment.
This all will go away on the next PR that removes the diagnostics binary. I'm trying to change the least i can and maintain functionality to make it easier to review.
| @@ -59,7 +59,7 @@ func (uc *UnifiedConfig) GenerateFilesFromConfig(ctx context.Context, service, l | |||
| } | |||
| } | |||
| case "otel": | |||
| otelConfig, err := uc.GenerateOtelConfig(ctx) | |||
| otelConfig, err := uc.GenerateOtelConfig(ctx, outDir) | |||
There was a problem hiding this comment.
[nit] In the fluent bit case, the Generate Command just returns the files and then we write in this function. For OTel though, it looks like we write the file in side this call. Lets make the behaviour consistent between the two?
There was a problem hiding this comment.
Can do on a follow up
There was a problem hiding this comment.
No, we don't write the otel generated configs inside GenerateOtelConfig, they are written in here the same way as the fluent-bit case (see the next 10 lines).
In the PR, @rafaelwestphal is passing outDir to know where did the *_oltp.json files were written too.
This reverts commit 0fff72d.
Description
Changing implementation of how self_metrics are reported.
This makes use of the enabled_receivers.json and feature_track.json files along with the otlpjsonfile receiver to push metrics instead of the using the diagnostics service.
Complete removal of the diagnostics service will be done in a coming PR.
Related issue
b/396458573
How has this been tested?
Checklist: