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 logging: bump lib and introduce pebble log forwarding #486

Merged
merged 4 commits into from
Aug 20, 2024

Conversation

sed-i
Copy link
Contributor

@sed-i sed-i commented May 27, 2024

Issue

Solution

  • Bump loki_push_api to v1.
  • Since air-gapped deployments are Juju 3.4+ anyway, use pebble log forwarding when possible.

Related to:

Testing

Deploy the bundle:

bundle: kubernetes
applications:
  loki:
    charm: loki-k8s
    channel: latest/edge
    base: [email protected]/stable
    resources:
      loki-image: 95
      node-exporter-image: 1
    scale: 1
    trust: true
  pg:
    charm: ./postgresql-k8s_ubuntu-22.04-amd64.charm
    resources:
      postgresql-image: ghcr.io/canonical/charmed-postgresql@sha256:31cf150b4523481202c1ff9b7b5d7f0b36729edad89d61242d8f1eb56b2912c0
    scale: 1
    trust: true
relations:
- - loki:logging
  - pg:logging

Confirm the pebble plan has a log-targets section:

juju exec --unit pg/0 -- PEBBLE_SOCKET=/charm/containers/postgresql/pebble.socket pebble plan
# ...
log-targets:
    loki/0:
        type: loki
        location: http://loki-0.loki-endpoints.pg.svc.cluster.local:3100/loki/api/v1/push
        services:
            - all
        override: replace
        labels:
            charm: postgresql-k8s
            juju_application: pg
            juju_model: pg
            juju_model_uuid: 31887e0e-4c62-48df-85db-7f529bc25177
            juju_unit: pg/0
            product: Juju

Confirms logs are in loki:

LOKI=$(juju status --format=json | jq -r '.applications.loki.units."loki/0".address')

curl -sG $LOKI:3100/loki/api/v1/labels
# {"status":"success","data":["charm","juju_application","juju_model","juju_model_uuid","juju_unit","pebble_service","product"]}

curl -sG $LOKI:3100/loki/api/v1/label/juju_unit/values
# {"status":"success","data":["pg/0"]}

curl -sG $LOKI:3100/loki/api/v1/query_range --data-urlencode 'query={pebble_service="postgresql"}' | jq '.data.result[0]'

Trimmed down output:

{
  "stream": {
    "charm": "postgresql-k8s",
    "juju_application": "pg",
    "juju_model": "pg",
    "juju_model_uuid": "31887e0e-4c62-48df-85db-7f529bc25177",
    "juju_unit": "pg/0",
    "pebble_service": "postgresql",
    "product": "Juju"
  },
  "values": [
    [
      "1716835072419000000",
      "server signaled"
    ],
    [
      "1716834778918000000",
      "2024-05-27 18:32:58 UTC [113]: user=,db=,app=,client=,line=2 HINT:  Future log output will appear in directory \"/var/log/postgresql\"."
    ],
    [
      "1716834777421000000",
      "performing post-bootstrap initialization ... ok"
    ],
    [
      "1716834777317000000",
      "The files belonging to this database system will be owned by user \"postgres\"."
    ]
  ]
}

@sed-i sed-i marked this pull request as draft May 27, 2024 19:26
self,
logs_scheme={"postgresql": {"log-files": POSTGRES_LOG_FILES}},
relation_name="logging",
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that, while LogProxyConsumer is streaming everything from

POSTGRES_LOG_FILES = [
    "/var/log/pgbackrest/*",
    "/var/log/postgresql/patroni.log",
    "/var/log/postgresql/postgresql*.log",
]

using LogForwarder streams the stdout of all pebble services.

The two may differ. If critical, then could add a pebble service analogue to tail -f the files.

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@sed-i sed-i May 31, 2024

Choose a reason for hiding this comment

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

It seems like there's a way to config pg to log to stdout, but I'm not clear on the syntax.
https://www.postgresql.org/docs/current/runtime-config-logging.html#GUC-LOGGING-COLLECTOR

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think a tailing service or stdout will work for Pgbackrest out of the box. IIRC it creates separate log files for various commands. I don't see an option to configure the logfile names. Maybe we can use --log-level-console and run the backup commands as a oneshot pebble service?

Copy link
Member

Choose a reason for hiding this comment

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

We probably can also use something like --log-level-console along with container.exec([pgbackrest-command])wait_output().

@sed-i sed-i changed the title Update logging to loki Update logging: bump lib and introduce pebble log forwarding May 27, 2024
Copy link

@lucabello lucabello left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link

codecov bot commented Aug 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.76%. Comparing base (a5af248) to head (01a5e94).
Report is 13 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #486      +/-   ##
==========================================
+ Coverage   70.71%   70.76%   +0.04%     
==========================================
  Files          11       11              
  Lines        2926     2931       +5     
  Branches      511      511              
==========================================
+ Hits         2069     2074       +5     
  Misses        748      748              
  Partials      109      109              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dragomirp
Copy link
Contributor

Hi, @sed-i, sorry for the long delay. I've merged current main to the branch and created #627 and #628 to track the outstanding log forwarding issues. I think we can merge this.

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.

5 participants