Skip to content

Limit number of logs collected #7

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

Open
wants to merge 10 commits into
base: stackhpc
Choose a base branch
from
Open

Limit number of logs collected #7

wants to merge 10 commits into from

Conversation

jovial
Copy link

@jovial jovial commented Mar 12, 2025

  • Adds the ability to set the logcount via query parameter or config option. Elasticsearch is better for long term storage.
  • Changes the default for log collection to false
    • I think this is a better default as collection is very slow. I'm also no convinced it was working before either.
  • Changed approach to using context parameter to make it easier to add extra query parameters in the future i.e we do not have to add a extra parameters to each function each time we want to add a new variable.
  • Made config options specific to groups. We could add the global option back if necessary.

This query is extremely slow, so limit to 10 logs. Elasticsearch
is better for long term storage.
@dougszumski dougszumski marked this pull request as draft March 14, 2025 13:18
@dougszumski
Copy link
Member

@jovial suggested extending this to make the number of logs collected configurable via a scrape parameter. Marking as draft.

@jovial
Copy link
Author

jovial commented Mar 17, 2025

@jovial suggested extending this to make the number of logs collected configurable via a scrape parameter. Marking as draft.

I did implement this, but it 2025/03/17 09:01:55 error error getting log entries from log service Manager=iDRAC.Embedded.1 app=redfish_exporter collector=ManagerCollector error=failed to retrieve some items: [{"link":"/redfish/v1/Managers/iDRAC.Embedded.1/LogServices/FaultList/Entries?$top=10","error":"501: {\n "error": {\n "code": "Base.1.0.GeneralError",\n "message": "A general error has occurred. See ExtendedInfo for more information.",\n "@Message.ExtendedInfo": [\n {\n "@odata.type" : "#Message.v1_0_0.Message",\n "MessageId": "Base.1.0.InternalError",\n "Message": "not supported, Query parameter top not supported"\n }\n some log services don't support the query parameter:


I'm going to split out the bit for duplicate metrics. Don't know the best way to handle that? Can we detect such a failure and query without it? Or make the option take a list of log services to apply the parameter to? 

@jovial
Copy link
Author

jovial commented Mar 17, 2025

Split out here: #8.

@jovial jovial changed the title Quick and dirty fix for slow log collection Limit number of logs collected Mar 17, 2025
@jovial jovial marked this pull request as ready for review March 31, 2025 08:30
targetLoggerCtx.WithError(err).Error("error parsing collectlogs query parameter as a boolean")
return
}
collectionCtx, err := common.NewCollectionContext(r, target, hostConfig, targetLoggerCtx)
Copy link
Member

@dougszumski dougszumski Apr 4, 2025

Choose a reason for hiding this comment

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

I like the addition of the context. From a quick glance, it now means that the redfish session is long lived? I.e. persists across scrapes. In that case, we need to handle re-authentication if the session expires / is deleted etc, or have I missed something?

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.

2 participants