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

[KODO-12017] 添加生产模式输出详细日志功能及配置 #499

Merged
merged 11 commits into from Apr 21, 2021
1 change: 1 addition & 0 deletions .gitignore
@@ -1,4 +1,5 @@
.DS_Store
.vscode
node_modules
bower_components
demo/config.js
Expand Down
9 changes: 4 additions & 5 deletions src/index.ts
@@ -1,11 +1,9 @@
import StatisticsLogger from './statisticsLog'
import createUploadManager, { Extra, Config, UploadOptions, UploadProgress } from './upload'
import { Observable, IObserver } from './observable'
import { CustomError } from './utils'
import { UploadCompleteData } from './api'
import compressImage from './compress'

const statisticsLogger = new StatisticsLogger()
import { Logger } from './logger'

/**
* @param file 上传文件
Expand All @@ -22,7 +20,6 @@ function upload(
putExtra?: Partial<Extra>,
config?: Partial<Config>
): Observable<UploadProgress, CustomError, UploadCompleteData> {

const options: UploadOptions = {
file,
key,
Expand All @@ -31,12 +28,14 @@ function upload(
config
}

// 为每个任务创建单独的 Logger
const logger = new Logger(token, config?.disableStatisticsReport, config?.debugLogLevel)
Copy link
Contributor

Choose a reason for hiding this comment

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

Logger constructor 的第二个参数名是 isReport,这边传的是 disable statistics report,是不是反了?

Copy link
Contributor

Choose a reason for hiding this comment

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

要不要为每个任务的日志带上唯一的标识(类似 tracing 系统的 req id),以便在同时存在多个上传任务时区分不同任务的输出

return new Observable((observer: IObserver<UploadProgress, CustomError, UploadCompleteData>) => {
const manager = createUploadManager(options, {
onData: (data: UploadProgress) => observer.next(data),
onError: (err: CustomError) => observer.error(err),
onComplete: (res: any) => observer.complete(res)
}, statisticsLogger)
}, logger)
manager.putFile()
return manager.stop.bind(manager)
})
Expand Down
72 changes: 72 additions & 0 deletions src/logger.ts
@@ -0,0 +1,72 @@
import { createXHR } from './utils'

export type LogLevel = '>=info' | '>=warn' | '>=error' | false
Copy link
Collaborator

Choose a reason for hiding this comment

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

补充文档
然后看一下对用户 & 代码来说,跟以前的 statisticsLog 定位有哪些是重合的、哪些是新增的
再看怎么调整
至少现在 程序日志 和 上报日志 现在更像是两种不同的日志,实现上不一定能 merge 成一个概念

Copy link
Contributor

Choose a reason for hiding this comment

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

NIP: 在我用过的 logger 的经验中,基本都约定 leve 设置为 INFO 就是表示 >= INFO,不用特别的 >=,参考 log4j、log4js、logrus 等

另外如果是希望关掉,有 LogLevel 'off' / 'OFF' 的做法,我觉得会比 false 稍好点


export class Logger {
lzfee0227 marked this conversation as resolved.
Show resolved Hide resolved
constructor(
private token: string,
private isReport = false,
private level: LogLevel = false,
) { }

private stack(): string | undefined {
const stack = new Error().stack?.split('\n')
// 裁掉前三行,显示真正的函数调用点
return stack?.slice(3)[0]?.trim()
Copy link
Contributor

Choose a reason for hiding this comment

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

如当面提到的,可能需要多验证一些浏览器;或者 stack 干脆不处理

}
Copy link
Collaborator

Choose a reason for hiding this comment

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

logger 自身不会报错吧。。?
@nighca 来看看。。


private report(logs: any[], retry = 3) {
if (!this.isReport) return

const xhr = createXHR()
xhr.open('POST', 'https://uplog.qbox.me/log/3')
yinxulai marked this conversation as resolved.
Show resolved Hide resolved
xhr.setRequestHeader('Content-type', 'application/x-www-form-urlencoded')
xhr.setRequestHeader('Authorization', 'UpToken ' + this.token)
xhr.onreadystatechange = () => {
if (xhr.readyState === 4 && xhr.status !== 200 && retry > 0) {
this.report(logs, retry - 1)
}
}

const formattedLogs = logs.map(log => {
try { return JSON.stringify(log) }
catch (error) { return JSON.stringify(error) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

如果这就是你的需求,那
简单 pick 一下有效信息,然后按需拼个可 stringify 的 error 出来,不要无脑 stringify?

但实际上,这应该不是你的需求?
这种情况,你要的其实不是这个 error,而是那个 log?
json.stringify 出异常了这件事本身是没有有效信息的,这时候你应该换一种方式提取并 stringify log 的信息?

你要不要干脆限制一下 logs 的类型 = =… 不要搞 any
这样交给输入方来处理
如果是普通的 Errorstring 或某种类似 KodoApiException 的你知道格式的东西,你这里是能处理的
但如果是复杂的情况,输入方比你这里更清楚,让输入方处理比你盲处理更合适?
限制一下类型,能一定程度上强制输入方处理好这件事?
剩下的情况,这里再兜底?

})

xhr.send(formattedLogs.join('\n'))
}

/**
* @param {any[]} ...args
Copy link
Contributor

Choose a reason for hiding this comment

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

NIP: TS 不用写 @param

* 输出 info 级别的调试信息
*/
info(...args: any[]) {
yinxulai marked this conversation as resolved.
Show resolved Hide resolved
const allowLevel: LogLevel[] = ['>=info']
if (allowLevel.includes(this.level)) {
console.log('Qiniu-JS-SDK[info]: ', ...args)
yinxulai marked this conversation as resolved.
Show resolved Hide resolved
}
}

/**
* @param {any[]} ...args
* 输出 warn 级别的调试信息
*/
warn(...args: any[]) {
const allowLevel: LogLevel[] = ['>=info', '>=warn']
if (allowLevel.includes(this.level)) {
console.warn('Qiniu-JS-SDK[warn]: ', ...args)
}
}

/**
* @param {any[]} ...args
* 输出 error 级别的调试信息
*/
error(...args: any[]) {
const allowLevel: LogLevel[] = ['>=info', '>=warn', '>=error']
if (allowLevel.includes(this.level)) {
console.error('Qiniu-JS-SDK[error]: ', ...args)
this.report([this.stack(), ...args])
}
}
}
54 changes: 35 additions & 19 deletions src/upload/base.ts
@@ -1,7 +1,7 @@
import * as utils from '../utils'
import { getUploadUrl, UploadCompleteData } from '../api'

import StatisticsLogger from '../statisticsLog'
import { Logger, LogLevel } from '../logger'
import { region } from '../config'

export const DEFAULT_CHUNK_SIZE = 4 // 单位 MB
Expand Down Expand Up @@ -32,14 +32,16 @@ export interface Config {
uphost: string
/** 自定义分片上传并发请求量 */
concurrentRequestLimit: number
/** 是否禁止静态日志上报 */
disableStatisticsReport: boolean
/** 分片大小,单位为 MB */
chunkSize: number
/** 上传域名协议 */
upprotocol: 'http:' | 'https:'
/** 上传区域 */
region?: typeof region[keyof typeof region]
/** 是否禁止静态日志上报 */
Copy link
Contributor

Choose a reason for hiding this comment

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

这个为啥叫”静态日志“来着..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修正为统计日志上报

yinxulai marked this conversation as resolved.
Show resolved Hide resolved
disableStatisticsReport: boolean
/** 设置调试日志输出模式,默认 false,不输出任何日志 */
yinxulai marked this conversation as resolved.
Show resolved Hide resolved
debugLogLevel?: LogLevel
yinxulai marked this conversation as resolved.
Show resolved Hide resolved
}

export interface UploadOptions {
Expand Down Expand Up @@ -103,7 +105,7 @@ export default abstract class Base {

protected abstract run(): utils.Response<any>

constructor(options: UploadOptions, handlers: UploadHandler, private statisticsLogger: StatisticsLogger) {
constructor(options: UploadOptions, handlers: UploadHandler, protected logger: Logger) {
this.config = {
useCdnDomain: true,
disableStatisticsReport: false,
Expand All @@ -117,69 +119,80 @@ export default abstract class Base {
...options.config
}

logger.info('inited Config', this.config)

this.putExtra = {
fname: '',
...options.putExtra
}

logger.info('inited putExtra', this.putExtra)

this.file = options.file
this.key = options.key
this.token = options.token

this.onData = handlers.onData
this.onError = handlers.onError
this.onComplete = handlers.onComplete

try {
this.bucket = utils.getPutPolicy(this.token).bucket
} catch (e) {
this.onError(e)
logger.error('get bucket from token failed.', e)
}
}

public async putFile(): Promise<utils.ResponseSuccess<UploadCompleteData>> {
public async putFile(): Promise<void> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

要不注释一下吧
说现在这个 promise 既不能告知成功与否,也不能说明结束与否
这种东西相对来说不那么常规

this.aborted = false
if (!this.putExtra.fname) {
this.logger.info('use file.name as fname.')
this.putExtra.fname = this.file.name
}

if (this.file.size > 10000 * GB) {
const err = new Error('file size exceed maximum value 10000G')
const errorMessage = 'file size exceed maximum value 10000G'
lzfee0227 marked this conversation as resolved.
Show resolved Hide resolved
const err = new Error(errorMessage)
this.logger.error(errorMessage)
this.onError(err)
throw err
}

if (this.putExtra.customVars) {
if (!utils.isCustomVarsValid(this.putExtra.customVars)) {
const err = new Error('customVars key should start width x:')
const errorMessage = 'customVars key should start width x:'
const err = new Error(errorMessage)
this.logger.error(errorMessage)
this.onError(err)
throw err
}
}

if (this.putExtra.metadata) {
if (!utils.isMetaDataValid(this.putExtra.metadata)) {
const err = new Error('metadata key should start with x-qn-meta-')
const errorMessage = 'metadata key should start with x-qn-meta-'
const err = new Error(errorMessage)
this.logger.error(errorMessage)
this.onError(err)
throw err
}
}

try {
this.logger.info('getUploadUrl')
this.uploadUrl = await getUploadUrl(this.config, this.token)
this.uploadAt = new Date().getTime()

const result = await this.run()
this.onComplete(result.data)

if (!this.config.disableStatisticsReport) {
this.sendLog(result.reqId, 200)
}

return result

this.sendLog(result.reqId, 200) // 收集成功的日志感觉没啥用?
lzfee0227 marked this conversation as resolved.
Show resolved Hide resolved
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

现在看来 按照原来的实现 我怎么觉得其实是这样的

this.putFIle().catch(err => {
  // 程序日志,会中断流程、抛到全局 & 打印到控制台,至于是否上报是另一回事?
  logger.error(err)
}) 

然后这里的 sendLog、以前的 statisticsLog 不是专门给前端在开发和使用的时候用的,但是前端也能看?
所以可能还是要跟 kodo 确认一下那个接收日志的后端接口拿到的数据现在谁在消费

} catch (err) {
this.logger.error(err)

this.clear()
if (err.isRequestError && !this.config.disableStatisticsReport) {
if (err.isRequestError) {
const reqId = this.aborted ? '' : err.reqId
const code = this.aborted ? -2 : err.code
this.sendLog(reqId, code)
Expand All @@ -191,20 +204,23 @@ export default abstract class Base {
// 1. 满足 needRetry 的条件且 retryCount 不为 0
// 2. uploadId 无效时在 resume 里会清除本地数据,并且这里触发重新上传
if (needRetry && notReachRetryCount || err.code === 612) {
this.logger.warn(`error auto retry: ${this.retryCount}/${this.config.retryCount}`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

这还真不好说是 warn 还是 info 其实

return this.putFile()
}

this.onError(err)
throw err
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MARK:所有的 error 都交给 onError,不应该再 throw 出去

Copy link
Collaborator

Choose a reason for hiding this comment

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

1、是否 throw 到 global 看一下 rxjs 的设定,比如是否跟绑定了 onError 有关
2、如果去掉这个改流程,那就要改 putFile 的定位,
还要检查所有调用 putFile 的地方,以及 putFile 内部所有 return throw 等流程控制点
现在明显有很多问题

Copy link
Contributor

Choose a reason for hiding this comment

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

要 at 恒哥来瞅一眼么..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

RxJS 的意思应该是如果传了 onError,所有的异常都会走 onError
https://github.com/ReactiveX/rxjs/blob/fa0471f96f1b9e8debe59c7eadda63b5b0aef5bd/src/internal/Observable.ts#L99

Copy link
Contributor

@winddies winddies Apr 19, 2021

Choose a reason for hiding this comment

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

这里我当时可能是想表达 rejected status,因为 putFile 本身是返回一个 promise, 既然出错了,那这个 promise 的状态如果是 resolved, 感觉就不太合适。
不过这个 promise 其实用户也不会用,只是内部的形式罢了,我看下面 catch 的 error 里也有 onError, 但是没有 throw, 这个应该是漏掉的,所以:

  1. 要么补加一下 throw,然后putFile 这里的 error 处理就统一了
  2. 把 throw 都去掉,不用关注 putFile 的 promise 状态,毕竟用户也不会用到

Copy link
Collaborator

Choose a reason for hiding this comment

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

putFile 本身是返回一个 promise, 既然出错了,那这个 promise 的状态如果是 resolved, 感觉就不太合适

跟你说的不太一样啊 @yinxulai

}
}

private clear() {
this.logger.info('start cleaning all xhr.')
this.xhrList.forEach(xhr => xhr.abort())
this.logger.info('cleanup completed.')
this.xhrList = []
}

public stop() {
this.logger.info('stop.')
this.clear()
this.aborted = true
}
Expand All @@ -214,7 +230,7 @@ export default abstract class Base {
}

private sendLog(reqId: string, code: number) {
this.statisticsLogger.log({
Copy link
Contributor

Choose a reason for hiding this comment

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

我看 logger 只有 error() 的内容才会发到服务端(report

所以以前这里的内容会记到服务端,现在不记到服务端了?

this.logger.info({
code,
reqId,
host: utils.getDomainFromUrl(this.uploadUrl),
Expand All @@ -225,7 +241,7 @@ export default abstract class Base {
bytesSent: this.progress ? this.progress.total.loaded : 0,
upType: 'jssdk-h5',
size: this.file.size
}, this.token)
})
}

public getProgressInfoItem(loaded: number, size: number) {
Expand Down
7 changes: 7 additions & 0 deletions src/upload/direct.ts
Expand Up @@ -5,6 +5,8 @@ import { UploadCompleteData } from '../api'
export default class Direct extends Base {

protected async run() {
this.logger.info('start run Direct.')

const formData = new FormData()
formData.append('file', this.file)
formData.append('token', this.token)
Expand All @@ -14,10 +16,13 @@ export default class Direct extends Base {
formData.append('fname', this.putExtra.fname)

if (this.putExtra.customVars) {
this.logger.info('inited customVars.')
lzfee0227 marked this conversation as resolved.
Show resolved Hide resolved
const { customVars } = this.putExtra
Object.keys(customVars).forEach(key => formData.append(key, customVars[key].toString()))
}

this.logger.info('inited formData.')
lzfee0227 marked this conversation as resolved.
Show resolved Hide resolved

const result = await request<UploadCompleteData>(this.uploadUrl, {
method: 'POST',
body: formData,
Expand All @@ -27,6 +32,7 @@ export default class Direct extends Base {
onCreate: xhr => this.addXhr(xhr)
})

this.logger.info('finishDirectProgress.')
lzfee0227 marked this conversation as resolved.
Show resolved Hide resolved
this.finishDirectProgress()
return result
}
Expand All @@ -40,6 +46,7 @@ export default class Direct extends Base {
private finishDirectProgress() {
// 在某些浏览器环境下,xhr 的 progress 事件无法被触发,progress 为 null,这里 fake 下
if (!this.progress) {
this.logger.warn('progress is null')
this.progress = { total: this.getProgressInfoItem(this.file.size, this.file.size) }
this.onData(this.progress)
return
Expand Down
17 changes: 11 additions & 6 deletions src/upload/index.ts
@@ -1,7 +1,7 @@
import Resume from './resume'
import Direct from './direct'
import { UploadOptions, UploadHandler } from './base'
import StatisticsLogger from '../statisticsLog'
import { Logger } from '../logger'
import { MB } from '../utils'

export * from './base'
Expand All @@ -10,13 +10,18 @@ export * from './resume'
export default function createUploadManager(
options: UploadOptions,
handlers: UploadHandler,
statisticsLogger: StatisticsLogger
logger: Logger
) {
if (options.config && options.config.forceDirect) {
return new Direct(options, handlers, statisticsLogger)
logger.info('ues forceDirect mode.')
return new Direct(options, handlers, logger)
}

return options.file.size > 4 * MB
? new Resume(options, handlers, statisticsLogger)
: new Direct(options, handlers, statisticsLogger)
if (options.file.size > 4 * MB) {
logger.info('file size over 4M, use Resume.')
return new Resume(options, handlers, logger)
}

logger.info('file size less than 4M, use Direct.')
Copy link
Contributor

Choose a reason for hiding this comment

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

NIP: less or equal

return new Direct(options, handlers, logger)
}