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

Make ParsePrometheusResults method public #259

Closed
wants to merge 1 commit into from

Conversation

cesnietor
Copy link
Contributor

We'll be using this method in other minIO products so sharing this so that we don't have to duplicate code. It already has tests.

Copy link
Member

@harshavardhana harshavardhana left a comment

Choose a reason for hiding this comment

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

I am not sure why this is implemented this way, we should fix the implementation first.

go func() {
		defer close(errChan)
		err = prom2json.ParseReader(reader, mfChan)
		if err != nil {
			errChan <- err
		}
	}()
	for mf := range mfChan {
		results = append(results, prom2json.NewFamily(mf))
	}
	if err := <-errChan; err != nil {
		return nil, err
	}
	return results, nil

There is no reason to have a go-routine for a blocking call.

Let's rewrite it like this clean and simple.

func ParsePrometheusResults(reader io.Reader) (results []*prom2json.Family, err error) {
        // We could do further content-type checks here, but the                                                                                                                                                   
        // fallback for now will anyway be the text format                                                                                                                                                         
        // version 0.0.4, so just go for it and see if it works.                                                                                                                                                   
        var parser expfmt.TextParser
        metricFamilies, err := parser.TextToMetricFamilies(reader)
        if err != nil {
                return nil, fmt.Errorf("reading text format failed: %v", err)
	}
        results = make([]*prom2json.Family, 0, len(metricFamilies))
	for _, mf := range metricFamilies {
		results = append(results, prom2json.NewFamily(mf))
	}
        return results, nil
}

@harshavardhana
Copy link
Member

@cesnietor ^^ PTAL

harshavardhana added a commit that referenced this pull request Jan 19, 2024
original work here #259,
re-implemented it to be idiomatic simpler API.
@harshavardhana
Copy link
Member

This is merged to the master with cleaner implementation.

harshavardhana added a commit that referenced this pull request Jan 19, 2024
original work here #259,
re-implemented it to be idiomatic simpler API.
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.

4 participants