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

Update log file names to be prefixed with the namespace #798

Open
Tracked by #47
jeromy-cannon opened this issue Nov 4, 2024 · 5 comments
Open
Tracked by #47

Update log file names to be prefixed with the namespace #798

jeromy-cannon opened this issue Nov 4, 2024 · 5 comments
Assignees
Labels
P0 An issue impacting production environments or impacting multiple releases or multiple individuals.

Comments

@jeromy-cannon
Copy link
Contributor

jeromy-cannon commented Nov 4, 2024

Currently, we have two log files on the users machine from which we run Solo:

  • ~/.solo/logs/solo.log
  • ~/.solo/logs/hashgraph-sdk.log

This will cause problems as we start to support using Solo for multiple deployments from the same machine.

I propose that we prefix the log file name with the namespace which matches the deployment. E.g.: namespace=solo-e2e

  • ~/.solo/logs/solo-e2e-solo.log
  • ~/.solo/logs/solo-e2e-hashgraph-sdk.log
@jeromy-cannon jeromy-cannon added the P0 An issue impacting production environments or impacting multiple releases or multiple individuals. label Nov 4, 2024
@JeffreyDallas
Copy link
Contributor

I found in indexts logger was instantiate first

const logger = logging.NewLogger('debug')`

then after manager, include configManager depends on logger, even before argv being parsed and set to flags.

Maybe we could just
do ~/.solo/<name-space/logs

even for other subdirectory cache we could put under ~/.solo<name-space/cache

so each name-space has its own dedicated directory for logs, caches

@jeromy-cannon
Copy link
Contributor Author

I'm okay with using the as a directory for logs and everything. as you mentioned:

~/.solo/<namespace>/logs/
~/.solo/<namespace>/cache/

I think to get the namespace you will need to either manually parse the argv (could be -n or --namespace, or you will need to run the yargs parsing engine against it using all flags. Not sure how much of a performance hit it is, to use yargs like this.

@JeffreyDallas
Copy link
Contributor

JeffreyDallas commented Nov 8, 2024

When calling solo cluster setup, it starts to dump logs to solo.log but at the moment, there is no user specific namespace
set yet, we could use clusterNamespace as prefix or subdirectory,
but then we may end up with two solo.log

<cluster_namespace>/solo.log
<user_namespace>solo.log

@jeromy-cannon
Copy link
Contributor Author

Jeromy Cannon
Today at 18:33
LocalConfig will have currentDeploymentName, which is the same as namespace,

currentDeploymentName : string

Also, see in my PR how I pull from argv early in main:#814

Jeffrey Tang
Today at 19:18
I don't think pull argv early would work, since we only set name space at network deploy step.
solo init
solo cluster setup
solo network deploy -n name_space
so when calling solo init or solo cluster setup we do not know the name space yet.
Maybe this issue 798 should wait until LocalConfig.ts get merged

Jeromy Cannon
Today at 19:27
in the future we will need to connect to or create a deployment first, which will set the namespace earlier. We can put a hold on this until after some of the other work is done if needed. Or, default to a /default/ directory if no namespace is found.

@JeffreyDallas
Copy link
Contributor

While doing some experiment, I found a tricky part.
The flags such as apiPermissionProperties or log4j2xml, those instances would instantiate
even before main function parsing any input arguments.
so their default values would be always using any old/default directory names.

export const apiPermissionProperties: CommandFlag = {
  constName: 'apiPermissionProperties',
  name: 'api-permission-properties',
  definition: {
    describe: 'api-permission.properties file for node',
    defaultValue: path.join(constants.SOLO_CACHE_DIR, 'templates', 'api-permission.properties'),
    type: 'string'
  }
}
export const apiPermissionProperties: CommandFlag = {
  constName: 'apiPermissionProperties',
  name: 'api-permission-properties',
  definition: {
    describe: 'api-permission.properties file for node',
    defaultValue: path.join(getCacheDirectory(), 'templates', 'api-permission.properties'),
    type: 'string'
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 An issue impacting production environments or impacting multiple releases or multiple individuals.
Projects
None yet
Development

No branches or pull requests

2 participants