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

[receiver/purefa] Add relabel and env configs to purefa #17266

Merged

Conversation

dgoscn
Copy link
Contributor

@dgoscn dgoscn commented Dec 27, 2022

Description:
Adds a new relabel config type to FlashArray receiver array name and the correspondent env.
eg: Relabels the host.name to the array name on array, pods, directory

Link to tracking Issue:
#14886

Testing:
Just follow the new config var on README, called env and check if his value is full filled

Documentation:
Still in progress

@runforesight
Copy link

runforesight bot commented Dec 27, 2022

Foresight Summary

    
Major Impacts

build-and-test-windows duration(42 seconds) has decreased 43 minutes 51 seconds compared to main branch avg(44 minutes 33 seconds).
View More Details

⭕  build-and-test-windows workflow has finished in 42 seconds (43 minutes 51 seconds less than main branch avg.) and finished at 9th Jan, 2023.


Job Failed Steps Tests
windows-unittest-matrix -     🔗  N/A See Details
windows-unittest -     🔗  N/A See Details

✅  tracegen workflow has finished in 7 minutes 47 seconds (⚠️ 4 minutes 41 seconds more than main branch avg.) and finished at 9th Jan, 2023.


Job Failed Steps Tests
publish-latest -     🔗  N/A See Details
publish-stable -     🔗  N/A See Details
build-dev -     🔗  N/A See Details

✅  changelog workflow has finished in 9 minutes 9 seconds and finished at 9th Jan, 2023.


Job Failed Steps Tests
changelog -     🔗  N/A See Details

✅  check-links workflow has finished in 9 minutes 14 seconds (⚠️ 6 minutes 32 seconds more than main branch avg.) and finished at 9th Jan, 2023.


Job Failed Steps Tests
changed files -     🔗  N/A See Details
check-links -     🔗  N/A See Details

✅  prometheus-compliance-tests workflow has finished in 10 minutes 3 seconds and finished at 9th Jan, 2023.


Job Failed Steps Tests
prometheus-compliance-tests -     🔗  ✅ 21  ❌ 0  ⏭ 0    🔗 See Details

 build-and-test workflow has finished in 18 minutes 51 seconds (28 minutes 21 seconds less than main branch avg.) and finished at 9th Jan, 2023.


Job Failed Steps Tests
unittest-matrix (1.18, internal) N/A  ✅ 619  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, internal) N/A  ✅ 619  ❌ 0  ⏭ 0    🔗 See Details
correctness-traces N/A  ✅ 17  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, processor) N/A  ✅ 1472  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, extension) N/A  ✅ 528  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, extension) N/A  ✅ 528  ❌ 0  ⏭ 0    🔗 See Details
correctness-metrics N/A  ✅ 2  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, processor) N/A  ✅ 1472  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, receiver-0) N/A  ✅ 2565  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, receiver-0) N/A  ✅ 2565  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, exporter) N/A  ✅ 2462  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, exporter) N/A  ✅ 2462  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, other) N/A  ✅ 4435  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.18, receiver-1) N/A  ✅ 1883  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, receiver-1) N/A  ✅ 1883  ❌ 0  ⏭ 0    🔗 See Details
integration-tests N/A  ✅ 55  ❌ 0  ⏭ 0    🔗 See Details
unittest-matrix (1.19, other) N/A  ✅ 4435  ❌ 0  ⏭ 0    🔗 See Details

✅  load-tests workflow has finished in 17 minutes 55 seconds and finished at 9th Jan, 2023.


Job Failed Steps Tests
loadtest (TestIdleMode) -     🔗  ✅ 1  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestTraceAttributesProcessor) -     🔗  ✅ 3  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestMetric10kDPS|TestMetricsFromFile) -     🔗  ✅ 6  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestTraceNoBackend10kSPS|TestTrace1kSPSWithAttrs) -     🔗  ✅ 8  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestMetricResourceProcessor|TestTrace10kSPS) -     🔗  ✅ 12  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestTraceBallast1kSPSWithAttrs|TestTraceBallast1kSPSAddAttrs) -     🔗  ✅ 10  ❌ 0  ⏭ 0    🔗 See Details
loadtest (TestBallastMemory|TestLog10kDPS) -     🔗  ✅ 19  ❌ 0  ⏭ 0    🔗 See Details
setup-environment -     🔗  N/A See Details

🔎 See details on Foresight

*You can configure Foresight comments in your organization settings page.

@dgoscn dgoscn changed the title [receiver/purefa] Add relabel and env configs to purefa on array names [receiver/purefa] Add relabel and env configs to purefa Dec 27, 2022
if scCfgs, err := podsScraper.ToPrometheusReceiverConfig(compHost, fact); err == nil {
scrapeCfgs = append(scrapeCfgs, scCfgs...)
} else {
return err
}

volumesScraper := internal.NewScraper(ctx, internal.ScraperTypeVolumes, r.cfg.Endpoint, r.cfg.Volumes, r.cfg.Settings.ReloadIntervals.Volumes)
volLabels := model.LabelSet{
Copy link
Member

Choose a reason for hiding this comment

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

If they all share the same label set, perhaps there could be only one var used across all NewScraper calls? But should they really have all the same label set? I had the impression that some would have fewer labels.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sounds reasonable about the var name across all NewScraper. I've checked on project resolutions and found that is only required for array, pods, directory and not for volumes, for instance. By the way, lets work on the only one var.

@@ -46,6 +46,9 @@ type Config struct {

// Volumes represents the list of volumes to query
Volumes []internal.ScraperConfig `mapstructure:"volumes"`

// Env represents the list of environment to query
Copy link
Member

Choose a reason for hiding this comment

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

If you are changing this PR for some reason, update this comment, as it's not a list of envs.

@jpkrohling
Copy link
Member

Looks like this still needs a changelog.

@dgoscn
Copy link
Contributor Author

dgoscn commented Jan 9, 2023

Looks like this still needs a changelog.

Sure, @jpkrohling. For some reason, I didn't saw your message.

Submitting asap

@jpkrohling jpkrohling merged commit 85ce958 into open-telemetry:main Jan 9, 2023
@chrroberts-pure
Copy link
Contributor

I am late to this party it seems. Sorry.

Env should be a setting per array, and not per endpoint.

@chrroberts-pure
Copy link
Contributor

We also need to consider how we will rewrite the label host from metrics from /metrics/hosts and /metrics/volumes

for instance this metric from /metrics/hosts

purefa_host_space_bytes{host="server01",space="total_provisioned"} 4.294967296e+09

we also need to add the fa_array_name label to those metrics as well.

we also need to add a config for adding a domain suffix to those hosts as well for host.name correlation purposes , because purefa doesn't support . in their hostnames.

@chrroberts-pure
Copy link
Contributor

let me know @dgoscn if you want me to create a new issue for the above

@dgoscn
Copy link
Contributor Author

dgoscn commented Jan 13, 2023

let me know @dgoscn if you want me to create a new issue for the above

Hello @chrroberts-pure sounds great. Can you please follow with the issue creation?

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants