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

[Feature Request]: log output path or log config path as environment variable or CLI option #1591

Closed
rudolfolah opened this issue Dec 27, 2023 · 3 comments
Labels
enhancement New feature or request

Comments

@rudolfolah
Copy link

Describe the problem

Currently, the log output path goes to chroma.log located near the chromadb_path.

Overriding it, based on the CLI code, means copying/pasting the log_config.yml and rewriting the configuration to point to a different logging path.

It would be great to be able to specify the exact path where the log file should be stored or to point to a different configuration file path.

Describe the proposed solution

Support CHROMA_LOG_OUTPUT_PATH as an environment variable or --log-output as a command-line argument

CHROMA_LOG_OUTPUT_PATH=/var/chroma.log chroma
chroma --log-output=/var/chroma.log

Alternatives considered

Alternatively, allow the log_config.yml path to be specified so that the configuration for logging can be completely customized:

CHROMA_LOG_CONFIG_PATH=/home/my-user/configs/chroma_log.yml chroma
chroma --log-config=/var/chroma/log_config.yml

Importance

would make my life easier

Additional Information

No response

@rudolfolah rudolfolah added the enhancement New feature or request label Dec 27, 2023
@tazarov
Copy link
Contributor

tazarov commented Dec 27, 2023

Hey @rudolfolah, I am glad you find the CLI useful. The team is looking to expand its functionality and usefulness, and what you suggest regarding the logging path falls within that category of improvements.

This should be pretty easy to add to the codebase, but let us spend a few moments considering the overall logging situation as this may also present an opportunity for improvement.

@rudolfolah
Copy link
Author

Thanks for the quick response, that sounds like a good plan.

And this may be useful to someone else, here's how I got it working with systemd:

chromadb.service systemd unit definition
[Unit]
Description=Chroma
After=network.target
StartLimitIntervalSec=0

[Service]
Type=exec
Restart=always
RestartSec=1
User=my-user
ExecStart=/home/my-user/.local/bin/chroma run --path '/home/my-user/chromadb' --port 8123
WorkingDirectory=/home/my-user
log_config.yml, goes into the WorkingDirectory from the chromadb.service configuration. Change the path in handlers.file.filename to output the log to another path.
version: 1
disable_existing_loggers: False
formatters:
  default:
    "()": uvicorn.logging.DefaultFormatter
    format: '%(levelprefix)s [%(asctime)s] %(message)s'
    use_colors: null
    datefmt: '%d-%m-%Y %H:%M:%S'
  access:
    "()": uvicorn.logging.AccessFormatter
    format: '%(levelprefix)s [%(asctime)s] %(client_addr)s - "%(request_line)s" %(status_code)s'
    datefmt: '%d-%m-%Y %H:%M:%S'
handlers:
  default:
    formatter: default
    class: logging.StreamHandler
    stream: ext://sys.stderr
  access:
    formatter: access
    class: logging.StreamHandler
    stream: ext://sys.stdout
  console:
    class: logging.StreamHandler
    stream: ext://sys.stdout
    formatter: default
  file:
    class : logging.handlers.RotatingFileHandler
    filename: /var/chroma.log
    formatter: default
loggers:
  root:
    level: WARN
    handlers: [console, file]
  chromadb:
    level: DEBUG
  uvicorn:
    level: INFO

tazarov added a commit to amikos-tech/chroma-core that referenced this issue Jan 14, 2024
HammadB pushed a commit that referenced this issue Jan 15, 2024
Refs: #1624, #1591

## Description of changes

*Summarize the changes made by this PR.*
 - Improvements & Bug fixes
- Added `--log--path` support to the CLI. A naive implementation that
works with Chroma's `log_config.yml`

## Test plan
*How are these changes tested?*

- [x] Tests pass locally with `pytest` for python

## Documentation Changes

TBD
@rudolfolah
Copy link
Author

Just saw the merged PR that references this issue, awesome work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants