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

Fix socket leak #55

Merged
merged 5 commits into from
Oct 6, 2020
Merged

Fix socket leak #55

merged 5 commits into from
Oct 6, 2020

Conversation

Nick-Triller
Copy link
Contributor

@Nick-Triller Nick-Triller commented Oct 2, 2020

Hi,

this MR fixes the socket leak reported in #52 and #45

Additionally, I refactored some of the code minimally. I took care to create succinct commits.

Thanks
Nick

@Nick-Triller Nick-Triller changed the title WIP: Fix socket leak Fix socket leak Oct 2, 2020
@dkulchinsky
Copy link
Contributor

Hey @Nick-Triller, just built from your branch and deployed in sandbox, but I started getting these errors:

level=error ts=2020-10-02T23:57:30.586Z caller=harbor_exporter.go:249 msg="Error handling request for /scans/all/metrics" http-statuscode="401 Unauthorized"
level=error ts=2020-10-02T23:57:30.586Z caller=metrics_scan.go:25 unexpectedendofJSONinput=(MISSING)
level=error ts=2020-10-02T23:57:32.140Z caller=harbor_exporter.go:249 msg="Error handling request for /statistics" http-statuscode="401 Unauthorized"
level=error ts=2020-10-02T23:57:32.140Z caller=metrics_statistics.go:27 unexpectedendofJSONinput=(MISSING)
level=error ts=2020-10-02T23:57:33.691Z caller=harbor_exporter.go:249 msg="Error handling request for /systeminfo/volumes" http-statuscode="401 Unauthorized"
level=error ts=2020-10-02T23:57:33.691Z caller=metrics_systemvolumes.go:21 unexpectedendofJSONinput=(MISSING)
level=error ts=2020-10-02T23:57:35.258Z caller=harbor_exporter.go:249 msg="Error handling request for /quotas?page=1&page_size=500" http-statuscode="401 Unauthorized"
level=error ts=2020-10-02T23:57:35.258Z caller=metrics_quotas.go:43 unexpectedendofJSONinput=(MISSING)
level=error ts=2020-10-02T23:57:36.823Z caller=harbor_exporter.go:249 msg="Error handling request for /projects?page=1&page_size=500" http-statuscode="422 Unprocessable Entity"
level=error ts=2020-10-02T23:57:36.823Z caller=metrics_repositories.go:64 unexpectedendofJSONinput=(MISSING)
level=error ts=2020-10-02T23:57:38.373Z caller=harbor_exporter.go:249 msg="Error handling request for /replication/policies?page=1&page_size=500" http-statuscode="401 Unauthorized"
level=error ts=2020-10-02T23:57:38.373Z caller=metrics_replications.go:39 msg="Error retrieving replication policies" err="unexpected end of JSON input"

The exporter is returning some of the metrics, but looks like some collectors are failing.

Any ideas?

@Nick-Triller
Copy link
Contributor Author

Hi @dkulchinsky, that's interesting, I am not seeing anything like that. Before I start looking into this, could you please test head of master, too?

@dkulchinsky
Copy link
Contributor

Hey @Nick-Triller, thanks for replying! my bad here... sorry for the noise 🤦 had an issue with password injection, so no more 401 errors.

though I'm confused why the exporter was still able to retrieve "some" of the data, I suppose some api endpoints do not require auth? anyway, will monitor memory usage for a while and report back.


this seems unrelated but was still getting this error on every scrape:

level=error ts=2020-10-05T13:12:30.165Z caller=harbor_exporter.go:249 msg="Error handling request for /projects?page=1&page_size=500" http-statuscode="422 Unprocessable Entity"
level=error ts=2020-10-05T13:12:30.165Z caller=metrics_repositories.go:64 unexpectedendofJSONinput=(MISSING)

managed to "fix" it by reducing HARBOR_PAGESIZE to 100 (defaults to 500), looking at the api swagger it does seem to suggest a maximum page size of 100 https://github.com/goharbor/harbor/blob/716625a76907aff8c9e5d7924d0c29fee26283d3/api/v2.0/swagger.yaml#L1499-L1507

  pageSize:
    name: page_size
    in: query
    type: integer
    format: int64
    required: false
    description: The size of per page
    default: 10
    maximum: 100

I'll open a separate issue for this.

@dkulchinsky
Copy link
Contributor

@Nick-Triller this has been running for 12 hours now and seems solid to me, great job 👍
image

@c4po would be awesome if this could be merged 🙏

@c4po
Copy link
Owner

c4po commented Oct 6, 2020

@Nick-Triller @dkulchinsky thanks for your help. I'll try to get on this tonight or tomorrow.

@c4po c4po changed the base branch from master to issue-52 October 6, 2020 13:20
@c4po c4po merged commit b88b5fb into c4po:issue-52 Oct 6, 2020
c4po added a commit that referenced this pull request Oct 6, 2020
* Fix socket leak (#55)

* Normalize method receivers

* Reuse HTTP client

* Remove unused code

* Fix typo in comment

* Rename variable

* bump the version to 0.5.7

Co-authored-by: Nick Triller <Nick-Triller@users.noreply.github.com>
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