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

Updated Configs With Field Best Practices #5

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

gpcarval
Copy link

  • Added specific scan folders for Linux and Unix to avoid issues with devices taking way too long to scan and generating big .snowpacks.
  • Added field best practices to all config files.
  • Normalized log levels to warning, instead of verbose.

Added specific scan folders for Linux and Unix to avoid issues with devices taking way too long to scan and generating big .snowpacks.
Added field best practices to all config files.
Normalized log levels to warning, instead of verbose.
Comment on lines 133 to 135
<MaxSize>2048</MaxSize>
<!-- error, warning, info, trace, verbose -->
<Level>verbose</Level>
<Level>warning</Level>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have to strongly object to this change. Experience shows that we'll have to ask for another log with verbose logging enabled, which will prolong resolution time and cause irritation on the customer side since they now have to replicate the problem.

Copy link
Author

Choose a reason for hiding this comment

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

I see your point, but running production with verbose logging is not ideal.
Althought we recycle the files to avoid build up, writting to file is "expensive" and can contribute to customer complaints on agent performance.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Have we actually had such complaints? If that is a thing then we can adjust the logging library to do buffered writes instead.

Copy link
Author

Choose a reason for hiding this comment

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

Never measured to be honest, but with the amount of data that we write, I do believe it could be a problem.
Not on Windows, but on big Linux/Unix boxes.

Maybe we can consider to run on Info to get some checkpoints of what is happening.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we have no actual complaints and have no actual facts pointing to it being a problem, why make support harder?

Copy link
Author

Choose a reason for hiding this comment

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

You have a good point.
I explained a bit as to why I'm against Verbose. Can you explain your reasoning for using it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gpcarval I think we have explained that already, what do you need clarification on?

Copy link
Author

Choose a reason for hiding this comment

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

Per, what did you see in your experience that makes you think having the agent running in verbose all the time helps at?
You mention response time, as we always ask for a verbose log in support. Do you have any information that backs that up?
Do we have a logging issue that we are trying to solve with verbose?
If the support engineer is going going to troubleshoot anyway, are we really making a difference with verbose?
Is our agent really that problematic that we need verbose all the time? If so, shouldn't we focus on fixing the underlying issue instead?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gpcarval Errors logs just the error itself, not what led up to it. I haven't documented all the times when we've asked for verbose logs, but feel free to ask support if they feel verbose logging gives additional information or not.

And yes, the logging issue is that there is an option to not log everything. imho.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gpcarval how to handle errors or warnings on such airtight systems, where people are not allowed to simply change the config and run the agent on demand or even access the system? I assume the costs in additional storage in a compressed snowpack is less then getting a proper log in the second run. People are literally using backup features, to have snowpacks at hand if support asks for such thing.

As this is just templates, just leave them at verbose. Can be reduced from the field, in alignment with what the customer thinks, his storage costs, vs the labour manually changing it on demand.

Comment on lines 123 to 125
<MaxSize>2048</MaxSize>
<!-- error, warning, info, trace, verbose -->
<Level>verbose</Level>
<Level>warning</Level>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have to strongly object to this change. Experience shows that we'll have to ask for another log with verbose logging enabled, which will prolong resolution time and cause irritation on the customer side since they now have to replicate the problem.

Comment on lines 99 to 101
<MaxSize>2048</MaxSize>
<!-- error, warning, info, trace, verbose -->
<Level>verbose</Level>
<Level>warning</Level>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have to strongly object to this change. Experience shows that we'll have to ask for another log with verbose logging enabled, which will prolong resolution time and cause irritation on the customer side since they now have to replicate the problem.

Comment on lines 38 to 40
<MaxSize>2048</MaxSize>
<!-- error, warning, info, trace, verbose -->
<Level>verbose</Level>
<Level>warning</Level>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have to strongly object to this change. Experience shows that we'll have to ask for another log with verbose logging enabled, which will prolong resolution time and cause irritation on the customer side since they now have to replicate the problem.

@snowliver
Copy link
Collaborator

changelog.md also needs an update.

Update Linux configuration based on feedback from Pull Request
-Removed explicit file system exclusion
-Removed "JDE and DB" path exclusions
-SSL verify is now set to True for security reasons
-Fixed identation
Changed the log level to reflect documentation
Update Unix configuration based on Pull Request feedback
-Removed default and invalid settings
-Changed log level to comply with documentation
-Fixed identation
-Fixed identation
-Changed scan schedule back to hh:mm:ss and added a comment to notify it uses 24h format
-Changed log level to comply with documentation
Copy link
Collaborator

@snowliver snowliver left a comment

Choose a reason for hiding this comment

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

please fix the indentation. the files do not use tabs but spaces.

<!-- Standard file system on Solaris 11 -->
<Setting key="software.scan.ips" value="true"/>
<!-- Standard file system on Solaris 10 and pervious versions -->
<Setting key="software.scan.srv4" value="true"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

svr4 would be the correct key.

@@ -120,5 +122,7 @@
<!-- <Setting key="env.java_home" value="java\bin\java.exe" /> -->
<!-- <Setting key="env.is_virtual_desktop_infrastructure" value="false" /> -->
<!-- <Setting key="disable_all_updates" value="false" /> -->
<!-- Control process affinity to limit CPU usage during scan-->
<!-- <Setting key="process_affinity" value="" /> -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we add this here, could we maybe use a valid value?

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gpcarval so why do we need to add an commented line with default value here? The config file should not just be the repetition of default values.

<Setting key="software.scan.srv4" value="true"/>
<!-- Standard file system on HP-UX -->
<Setting key="software.scan.sd" value="true"/>
<!-- Attempt to find detect autofs mounts and add these to the list of directories to be ignored -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

would go for 'detect' - pls also change in the linux config.

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