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

use Symbol.for('level') when extracting label #112

Closed
2 tasks done
jburghardt opened this issue Jun 8, 2022 · 1 comment · Fixed by #144
Closed
2 tasks done

use Symbol.for('level') when extracting label #112

jburghardt opened this issue Jun 8, 2022 · 1 comment · Fixed by #144
Labels

Comments

@jburghardt
Copy link

jburghardt commented Jun 8, 2022

following line extract the level information from the info object and uses them as the level label

https://github.com/JaniAnttonen/winston-loki/blob/development/index.js#L58

this causes problems when using

  • colorized text
  • text that is padded for formation
  • levels that are lower or uppercase

instead the Symbol.for('level') should be used, as per docu of winston::
https://github.com/winstonjs/winston#streams-objectmode-and-info-objects

Checklist
  • Modify index.jse35241a Edit
  • Running GitHub Actions for index.jsEdit
Copy link
Contributor

sweep-ai bot commented Mar 29, 2024

🚀 Here's the PR! #144

See Sweep's progress at the progress dashboard!
💎 Sweep Pro: I'm using GPT-4. You have unlimited GPT-4 tickets. (tracking ID: b1700abefc)
Install Sweep Configs: Pull Request

Tip

I can email you next time I complete a pull request if you set up your email here!


Actions (click)

  • ↻ Restart Sweep

Step 1: 🔎 Searching

I found the following snippets in your repository. I will now analyze these snippets and come up with a plan.

Some code snippets I think are relevant in decreasing order of relevance (click to expand). If some file is missing from here, you can mention the path in the ticket description.

winston-loki/index.js

Lines 47 to 104 in 88399c8

*/
log (info, callback) {
// Immediately tell Winston that this transport has received the log.
setImmediate(() => {
this.emit('logged', info)
})
// Deconstruct the log
const { label, labels, timestamp, level, message, ...rest } = info
// build custom labels if provided
let lokiLabels = { level: level }
if (this.labels) {
lokiLabels = Object.assign(lokiLabels, this.labels)
} else {
lokiLabels.job = label
}
lokiLabels = Object.assign(lokiLabels, labels)
// follow the format provided
const line = this.useCustomFormat
? info[MESSAGE]
: `${message} ${
rest && Object.keys(rest).length > 0 ? JSON.stringify(rest) : ''
}`
// Make sure all label values are strings
lokiLabels = Object.fromEntries(Object.entries(lokiLabels).map(([key, value]) => [key, value ? value.toString() : value]))
// Construct the log to fit Grafana Loki's accepted format
let ts
if (timestamp) {
ts = new Date(timestamp)
ts = isNaN(ts) ? Date.now() : ts.valueOf()
} else {
ts = Date.now()
}
const logEntry = {
labels: lokiLabels,
entries: [
{
ts,
line
}
]
}
// Pushes the log to the batcher
this.batcher.pushLogEntry(logEntry).catch(err => {
// eslint-disable-next-line no-console
console.error(err)
})
// Trigger the optional callback
callback()


Step 2: ⌨️ Coding

Modify index.js with contents:
• In the `log` function within `index.js`, locate the line where the `level` is being extracted from the `info` object (currently line 55).
• Replace the direct access of the `level` property with the use of `Symbol.for('level')` to extract the log level. This involves changing the line from: ```javascript const { label, labels, timestamp, level, message, ...rest } = info ``` to: ```javascript const { label, labels, timestamp, message, ...rest } = info const level = info[Symbol.for('level')] ```
• This change ensures that the log level is extracted in a manner that is compatible with Winston's handling of log levels, addressing the issue of colorized text, padded formatting, and case sensitivity of log levels.
• No other changes are required in `index.js` or any other files to resolve the reported issue.
--- 
+++ 
@@ -52,7 +52,8 @@
     })
 
     // Deconstruct the log
-    const { label, labels, timestamp, level, message, ...rest } = info
+    const { label, labels, timestamp, message, ...rest } = info
+    const level = info[Symbol.for('level')]
 
     // build custom labels if provided
     let lokiLabels = { level: level }
  • Running GitHub Actions for index.jsEdit
Check index.js with contents:

Ran GitHub Actions for e35241a2c43688fb0126865fc9f43fc8bee3a257:


Step 3: 🔁 Code Review

I have finished reviewing the code for completeness. I did not find errors for sweep/use_symbolforlevel_when_extracting_label.


🎉 Latest improvements to Sweep:
  • New dashboard launched for real-time tracking of Sweep issues, covering all stages from search to coding.
  • Integration of OpenAI's latest Assistant API for more efficient and reliable code planning and editing, improving speed by 3x.
  • Use the GitHub issues extension for creating Sweep issues directly from your editor.

💡 To recreate the pull request edit the issue title or description.
Something wrong? Let us know.

This is an automated message generated by Sweep AI.

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

Successfully merging a pull request may close this issue.

2 participants