-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Add new elasticsearch client #69905
Add new elasticsearch client #69905
Changes from 3 commits
10f0846
e923ce5
175e0cb
f56a506
24ac36b
024f3df
cfa547a
eecd780
8c6c9fc
c7a2cf1
193fecc
b8935d7
f268e63
73f68b8
2c5a489
c7ae6aa
93d05ee
4156411
5a3fd9c
71a27bb
e48f3ad
bdacda3
152b6dc
b265759
9e356e8
353253f
7e49f65
bda204e
28b8cfc
8cd1ed2
205c100
7819166
d1cdb0a
80bfc97
bf18e61
6c4ff93
8a397d0
533212b
0a09aee
1a68ea1
f956fb4
56db6c4
d6f1c9d
83b7246
a764b10
9d272ba
3f1d9bd
88ae350
cb242e7
413a97d
c0d7f3a
022e4a2
0dc69b4
5acfe96
f059964
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,145 @@ | ||
/* | ||
* Licensed to Elasticsearch B.V. under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch B.V. licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import { ConnectionOptions as TlsConnectionOptions } from 'tls'; | ||
import { URL } from 'url'; | ||
import { Duration } from 'moment'; | ||
import { ClientOptions, NodeOptions } from '@elastic/elasticsearch'; | ||
import { ElasticsearchConfig } from '../elasticsearch_config'; | ||
|
||
/** | ||
* @privateRemarks Config that consumers can pass to the Elasticsearch JS client is complex and includes | ||
* not only entries from standard `elasticsearch.*` yaml config, but also some Elasticsearch JS | ||
* client specific options like `keepAlive` or `plugins` (that eventually will be deprecated). | ||
* | ||
* @public | ||
*/ | ||
export type ElasticsearchClientConfig = Pick< | ||
ElasticsearchConfig, | ||
| 'customHeaders' | ||
| 'logQueries' | ||
| 'sniffOnStart' | ||
| 'sniffOnConnectionFault' | ||
| 'requestHeadersWhitelist' | ||
| 'sniffInterval' | ||
| 'hosts' | ||
| 'username' | ||
| 'password' | ||
> & { | ||
pingTimeout?: ElasticsearchConfig['pingTimeout'] | ClientOptions['pingTimeout']; | ||
requestTimeout?: ElasticsearchConfig['requestTimeout'] | ClientOptions['requestTimeout']; | ||
ssl?: Partial<ElasticsearchConfig['ssl']>; | ||
}; | ||
|
||
export function parseClientOptions( | ||
config: ElasticsearchClientConfig, | ||
scoped: boolean | ||
): ClientOptions { | ||
const clientOptions: ClientOptions = { | ||
sniffOnStart: config.sniffOnStart, | ||
sniffOnConnectionFault: config.sniffOnConnectionFault, | ||
headers: config.customHeaders, | ||
pgayvallet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}; | ||
|
||
if (config.pingTimeout != null) { | ||
clientOptions.pingTimeout = getDurationAsMs(config.pingTimeout); | ||
} | ||
if (config.requestTimeout != null) { | ||
clientOptions.requestTimeout = getDurationAsMs(config.requestTimeout); | ||
} | ||
if (config.sniffInterval) { | ||
clientOptions.sniffInterval = getDurationAsMs(config.sniffInterval); | ||
} | ||
|
||
// TODO: this can either be done globally here or by host in convertHost. | ||
// Not sure which option is the best. | ||
if (config.username && config.password) { | ||
clientOptions.auth = { | ||
username: config.username, | ||
password: config.password, | ||
}; | ||
} | ||
|
||
clientOptions.nodes = config.hosts.map((host) => convertHost(host, !scoped, config)); | ||
|
||
if (config.ssl) { | ||
clientOptions.ssl = generateSslConfig( | ||
config.ssl, | ||
scoped && !config.ssl.alwaysPresentCertificate | ||
); | ||
} | ||
|
||
return clientOptions; | ||
} | ||
|
||
const generateSslConfig = ( | ||
sslConfig: Required<ElasticsearchClientConfig>['ssl'], | ||
ignoreCertAndKey: boolean | ||
): TlsConnectionOptions => { | ||
const ssl: TlsConnectionOptions = { | ||
ca: sslConfig.certificateAuthorities, | ||
}; | ||
|
||
const verificationMode = sslConfig.verificationMode; | ||
switch (verificationMode) { | ||
case 'none': | ||
ssl.rejectUnauthorized = false; | ||
break; | ||
case 'certificate': | ||
ssl.rejectUnauthorized = true; | ||
// by default, NodeJS is checking the server identify | ||
ssl.checkServerIdentity = () => undefined; | ||
break; | ||
case 'full': | ||
ssl.rejectUnauthorized = true; | ||
break; | ||
default: | ||
throw new Error(`Unknown ssl verificationMode: ${verificationMode}`); | ||
} | ||
|
||
// Add client certificate and key if required by elasticsearch | ||
if (!ignoreCertAndKey && sslConfig.certificate && sslConfig.key) { | ||
ssl.cert = sslConfig.certificate; | ||
ssl.key = sslConfig.key; | ||
ssl.passphrase = sslConfig.keyPassphrase; | ||
} | ||
|
||
return ssl; | ||
}; | ||
|
||
const convertHost = ( | ||
host: string, | ||
needAuth: boolean, | ||
{ username, password }: ElasticsearchClientConfig | ||
): NodeOptions => { | ||
const url = new URL(host); | ||
const isHTTPS = url.protocol === 'https:'; | ||
url.port = url.port ?? isHTTPS ? '443' : '80'; | ||
if (needAuth && username && password) { | ||
url.username = username; | ||
url.password = password; | ||
} | ||
|
||
return { | ||
url, | ||
}; | ||
}; | ||
|
||
const getDurationAsMs = (duration: number | Duration) => | ||
typeof duration === 'number' ? duration : duration.asMilliseconds(); |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,57 @@ | ||||||||||||||||||||||||||||||||||||||||
/* | ||||||||||||||||||||||||||||||||||||||||
* Licensed to Elasticsearch B.V. under one or more contributor | ||||||||||||||||||||||||||||||||||||||||
* license agreements. See the NOTICE file distributed with | ||||||||||||||||||||||||||||||||||||||||
* this work for additional information regarding copyright | ||||||||||||||||||||||||||||||||||||||||
* ownership. Elasticsearch B.V. licenses this file to you under | ||||||||||||||||||||||||||||||||||||||||
* the Apache License, Version 2.0 (the "License"); you may | ||||||||||||||||||||||||||||||||||||||||
* not use this file except in compliance with the License. | ||||||||||||||||||||||||||||||||||||||||
* You may obtain a copy of the License at | ||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||
* http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||
* Unless required by applicable law or agreed to in writing, | ||||||||||||||||||||||||||||||||||||||||
* software distributed under the License is distributed on an | ||||||||||||||||||||||||||||||||||||||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||||||||||||||||||||||||||||||||||||||||
* KIND, either express or implied. See the License for the | ||||||||||||||||||||||||||||||||||||||||
* specific language governing permissions and limitations | ||||||||||||||||||||||||||||||||||||||||
* under the License. | ||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
import { ApiResponse } from '@elastic/elasticsearch'; | ||||||||||||||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||||||||||||||
RequestBody, | ||||||||||||||||||||||||||||||||||||||||
RequestNDBody, | ||||||||||||||||||||||||||||||||||||||||
TransportRequestOptions, | ||||||||||||||||||||||||||||||||||||||||
TransportRequestPromise, | ||||||||||||||||||||||||||||||||||||||||
} from '@elastic/elasticsearch/lib/Transport'; | ||||||||||||||||||||||||||||||||||||||||
import * as RequestParams from '@elastic/elasticsearch/api/requestParams'; | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
export interface ClientFacade { | ||||||||||||||||||||||||||||||||||||||||
bulk< | ||||||||||||||||||||||||||||||||||||||||
TResponse = Record<string, any>, | ||||||||||||||||||||||||||||||||||||||||
TRequestBody extends RequestNDBody = Array<Record<string, any>>, | ||||||||||||||||||||||||||||||||||||||||
TContext = unknown | ||||||||||||||||||||||||||||||||||||||||
>( | ||||||||||||||||||||||||||||||||||||||||
params?: RequestParams.Bulk<TRequestBody>, | ||||||||||||||||||||||||||||||||||||||||
options?: TransportRequestOptions | ||||||||||||||||||||||||||||||||||||||||
): TransportRequestPromise<ApiResponse<TResponse, TContext>>; | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the successor of Most important question is: Do we want to expose the As a reminder, export interface TransportRequestOptions {
ignore?: number[];
requestTimeout?: number | string;
maxRetries?: number;
asStream?: boolean;
headers?: Record<string, any>;
querystring?: Record<string, any>;
compression?: 'gzip';
id?: any;
context?: any;
warnings?: string[];
opaqueId?: string;
} I don't think I have enough knowledge of our usages of the ES client to decide on this one. @rudolf maybe? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
IIRC ll the types are auto-generated. It's error-prone to update them manually every time we bump the library version. Can we just re-use the same typings?
I don't see why we shouldn't. We already provide There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The generated types are a mess (take a look at https://github.com/elastic/elasticsearch-js/blob/master/index.d.ts) I would love to avoid replicating what we did with I.E These are the signatures for delete<TResponse = Record<string, any>, TContext = unknown>(params?: RequestParams.AsyncSearchDelete, options?: TransportRequestOptions): TransportRequestPromise<ApiResponse<TResponse, TContext>>
delete<TResponse = Record<string, any>, TContext = unknown>(callback: callbackFn<TResponse, TContext>): TransportRequestCallback
delete<TResponse = Record<string, any>, TContext = unknown>(params: RequestParams.AsyncSearchDelete, callback: callbackFn<TResponse, TContext>): TransportRequestCallback
delete<TResponse = Record<string, any>, TContext = unknown>(params: RequestParams.AsyncSearchDelete, options: TransportRequestOptions, callback: callbackFn<TResponse, TContext>): TransportRequestCallback Second point, if we use the client's signatures instead of replicating them, we would never be able to introduce higher level options that are consumed by our wrapper (as that was done with Other (minor) point, all the apis are available both in camel and snake case. It would be great to avoid such pollution, and that would also avoid having to grep for two things when searching for usages (this one could be resolved with a delete_autoscaling_policy<TResponse = Record<string, any>, TContext = unknown>(params?: RequestParams.AutoscalingDeleteAutoscalingPolicy, options?: TransportRequestOptions): TransportRequestPromise<ApiResponse<TResponse, TContext>>
delete_autoscaling_policy<TResponse = Record<string, any>, TContext = unknown>(callback: callbackFn<TResponse, TContext>): TransportRequestCallback
delete_autoscaling_policy<TResponse = Record<string, any>, TContext = unknown>(params: RequestParams.AutoscalingDeleteAutoscalingPolicy, callback: callbackFn<TResponse, TContext>): TransportRequestCallback
delete_autoscaling_policy<TResponse = Record<string, any>, TContext = unknown>(params: RequestParams.AutoscalingDeleteAutoscalingPolicy, options: TransportRequestOptions, callback: callbackFn<TResponse, TContext>): TransportRequestCallback
deleteAutoscalingPolicy<TResponse = Record<string, any>, TContext = unknown>(params?: RequestParams.AutoscalingDeleteAutoscalingPolicy, options?: TransportRequestOptions): TransportRequestPromise<ApiResponse<TResponse, TContext>>
deleteAutoscalingPolicy<TResponse = Record<string, any>, TContext = unknown>(callback: callbackFn<TResponse, TContext>): TransportRequestCallback
deleteAutoscalingPolicy<TResponse = Record<string, any>, TContext = unknown>(params: RequestParams.AutoscalingDeleteAutoscalingPolicy, callback: callbackFn<TResponse, TContext>): TransportRequestCallback
deleteAutoscalingPolicy<TResponse = Record<string, any>, TContext = unknown>(params: RequestParams.AutoscalingDeleteAutoscalingPolicy, options: TransportRequestOptions, callback: callbackFn<TResponse, TContext>): TransportRequestCallback Third point, in my opinion again, in term of Dev experience, A There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That's true, the client supports all possible use-cases which we don't want to. I'm still skeptical about manual work required on every update... That's not ideal, but we can adjust type generator script in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Automated generation could definitely be an option if we are afraid of manual maintenance when we bump the library. So we would use (and maintain) an edited version of their script to generate our (currently named) I can give this a try if we want to. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Just saw that the Maybe AST parsing of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, after
So, instead, I moved on using a (way less sexy but yet effective for our needs) plain regexp-based parsing of their I feel like this could do the trick, wdyt? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
😅 On the whole I'm okay even to have a manual process.
Ok, as long as it works. I didn't review the whole file. I thought that we could extend the script right in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be simpler to change the script in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @joshdover sorry, you were not included in the slack discussion between delvedor, restry and myself. A brief summary: Delvedor did that for us (
Overall, imho, these generation scripts 'just works (tm)' for our need, at least for now. As it's just an implementation detail (it shouldnt impact core' public API), I'd say we could probably go with it on the initial implementation, and eventually change the approach later. |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
asyncSearch: { | ||||||||||||||||||||||||||||||||||||||||
delete<TResponse = Record<string, any>, TContext = unknown>( | ||||||||||||||||||||||||||||||||||||||||
params?: RequestParams.AsyncSearchDelete, | ||||||||||||||||||||||||||||||||||||||||
options?: TransportRequestOptions | ||||||||||||||||||||||||||||||||||||||||
): TransportRequestPromise<ApiResponse<TResponse, TContext>>; | ||||||||||||||||||||||||||||||||||||||||
get<TResponse = Record<string, any>, TContext = unknown>( | ||||||||||||||||||||||||||||||||||||||||
params?: RequestParams.AsyncSearchGet, | ||||||||||||||||||||||||||||||||||||||||
options?: TransportRequestOptions | ||||||||||||||||||||||||||||||||||||||||
): TransportRequestPromise<ApiResponse<TResponse, TContext>>; | ||||||||||||||||||||||||||||||||||||||||
submit< | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another 'detail' regarding this typed/structured replacement of kibana/src/core/server/elasticsearch/retry_call_cluster.ts Lines 89 to 107 in bf04235
Previously Maybe someone have an idea? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can change signature to accept a function: export function retryCallCluster(fn: () => Promise<T>): Promise<T> {
return defer(fn())
.pipe(
retryWhen((errors) =>
errors.pipe(
concatMap((error, i) =>
iif(
() => error instanceof legacyElasticsearch.errors.NoConnections,
timer(1000),
throwError(error)
)
)
)
)
)
.toPromise();
};
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, that's the 'easier' solution I see. However multiple endpoint/API calls are used in the SO client. wrapping the whole APICaller with that allowed to be sure every call was going to be wrapper with the retry logic. If we wrap each individual methods, we would need to adapt all calls in the SO repository that were based on this retry logic. Not really seeing another option atm though. |
||||||||||||||||||||||||||||||||||||||||
TResponse = Record<string, any>, | ||||||||||||||||||||||||||||||||||||||||
TRequestBody extends RequestBody = Record<string, any>, | ||||||||||||||||||||||||||||||||||||||||
TContext = unknown | ||||||||||||||||||||||||||||||||||||||||
>( | ||||||||||||||||||||||||||||||||||||||||
params?: RequestParams.AsyncSearchSubmit<TRequestBody>, | ||||||||||||||||||||||||||||||||||||||||
options?: TransportRequestOptions | ||||||||||||||||||||||||||||||||||||||||
): TransportRequestPromise<ApiResponse<TResponse, TContext>>; | ||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
/* | ||
* Licensed to Elasticsearch B.V. under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch B.V. licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import { Client } from '@elastic/elasticsearch'; | ||
import { TransportRequestOptions } from '@elastic/elasticsearch/lib/Transport'; | ||
import { Headers } from '../../http/router'; | ||
import { ClientFacade } from './client_facade'; | ||
|
||
export const getClientFacade = (client: Client, headers: Headers = {}): ClientFacade => { | ||
const addHeaders = (options?: TransportRequestOptions): TransportRequestOptions => { | ||
if (!options) { | ||
return { | ||
headers, | ||
}; | ||
} | ||
// TODO: do we need to throw in case of duplicates as it was done | ||
// in legacy? - src/core/server/elasticsearch/scoped_cluster_client.ts:L88 | ||
return { | ||
...options, | ||
headers: { | ||
...options.headers, | ||
...headers, | ||
}, | ||
}; | ||
pgayvallet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}; | ||
|
||
return { | ||
bulk: (params, options) => client.bulk(params, addHeaders(options)), | ||
asyncSearch: { | ||
delete: (params, options) => client.asyncSearch.delete(params, addHeaders(options)), | ||
get: (params, options) => client.asyncSearch.get(params, addHeaders(options)), | ||
submit: (params, options) => client.asyncSearch.submit(params, addHeaders(options)), | ||
}, | ||
}; | ||
pgayvallet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
/* | ||
* Licensed to Elasticsearch B.V. under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch B.V. licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import { Client } from '@elastic/elasticsearch'; | ||
import { getClientFacade } from './client_wrapper'; | ||
import { ClientFacade } from './client_facade'; | ||
import { configureClient } from './configure_client'; | ||
import { Logger } from '../../logging'; | ||
import { GetAuthHeaders, isRealRequest } from '../../http'; | ||
import { Headers } from '../../http/router'; | ||
import { ElasticsearchClientConfig } from './client_config'; | ||
import { ScopedClusterClient, IScopedClusterClient } from './scoped_cluster_client'; | ||
import { ScopeableRequest } from './types'; | ||
import { ensureRawRequest, filterHeaders } from '../../http/router'; | ||
|
||
const noop = () => undefined; | ||
|
||
interface IClusterClient { | ||
asInternalUser: () => ClientFacade; | ||
asScoped: (request: ScopeableRequest) => IScopedClusterClient; | ||
pgayvallet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
export class ClusterClient implements IClusterClient { | ||
private readonly internalWrapper: ClientFacade; | ||
private readonly scopedClient: Client; | ||
|
||
constructor( | ||
private readonly config: ElasticsearchClientConfig, | ||
logger: Logger, | ||
private readonly getAuthHeaders: GetAuthHeaders = noop | ||
) { | ||
this.internalWrapper = getClientFacade(configureClient(config, { logger })); | ||
this.scopedClient = configureClient(config, { logger, scoped: true }); | ||
pgayvallet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
asInternalUser() { | ||
return this.internalWrapper; | ||
} | ||
|
||
asScoped(request: ScopeableRequest) { | ||
const headers = this.getScopedHeaders(request); | ||
const scopedWrapper = getClientFacade(this.scopedClient, headers); | ||
return new ScopedClusterClient(this.internalWrapper, scopedWrapper); | ||
} | ||
mshustov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
private getScopedHeaders(request: ScopeableRequest): Headers { | ||
if (!isRealRequest(request)) { | ||
return request?.headers ?? {}; | ||
} | ||
const authHeaders = this.getAuthHeaders(request); | ||
const headers = ensureRawRequest(request).headers; | ||
|
||
return filterHeaders({ ...headers, ...authHeaders }, this.config.requestHeadersWhitelist); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,46 @@ | ||||
/* | ||||
* Licensed to Elasticsearch B.V. under one or more contributor | ||||
* license agreements. See the NOTICE file distributed with | ||||
* this work for additional information regarding copyright | ||||
* ownership. Elasticsearch B.V. licenses this file to you under | ||||
* the Apache License, Version 2.0 (the "License"); you may | ||||
* not use this file except in compliance with the License. | ||||
* You may obtain a copy of the License at | ||||
* | ||||
* http://www.apache.org/licenses/LICENSE-2.0 | ||||
* | ||||
* Unless required by applicable law or agreed to in writing, | ||||
* software distributed under the License is distributed on an | ||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||||
* KIND, either express or implied. See the License for the | ||||
* specific language governing permissions and limitations | ||||
* under the License. | ||||
*/ | ||||
|
||||
import { Client } from '@elastic/elasticsearch'; | ||||
import { Logger } from '../../logging'; | ||||
import { parseClientOptions, ElasticsearchClientConfig } from './client_config'; | ||||
|
||||
export const configureClient = ( | ||||
config: ElasticsearchClientConfig, | ||||
{ logger, scoped = false }: { logger: Logger; scoped?: boolean } | ||||
): Client => { | ||||
const clientOptions = parseClientOptions(config, scoped); | ||||
const client = new Client(clientOptions); | ||||
|
||||
client.on('response', (err, event) => { | ||||
if (err) { | ||||
logger.error(`${err.name}: ${err.message}`); | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we okay not to log
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I took #69905 (comment) as a 'yes', as warning are per-request and can now be handled by the consumers. but we can decide to log them on our own. I don't really have an opinion on that one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’m wondering if the client should log warnings more aggressively, maybe via There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||
} else if (config.logQueries) { | ||||
const params = event.meta.request.params; | ||||
logger.debug( | ||||
`${event.statusCode}\n${params.method} ${params.path}\n${params.querystring?.trim() ?? ''}`, | ||||
{ | ||||
tags: ['query'], | ||||
} | ||||
); | ||||
} | ||||
}); | ||||
pgayvallet marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
|
||||
return client; | ||||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The client follows the stack versioning, meaning that using the client
7.x
in kibanamaster
will cause issues. You should use the clientmaster
branch.Currently, we are not publishing any
8.x
version on npm, but we could do it if it does help you.Here you can find the compatibility table of the client.
If you want to install the master branch of the client:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the client released separately for every Stack release? Should it be another place to sync across the Stack when bumping a Kibana version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we do a release for every
major.minor
of the stack, patches are released as soon as it's needed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, this may be quite problematic AFAIK, as kibana master is targeting 8.0, but the current branch (7.9 atm for example) is targeting 7.X
That means that we would need to have different versions (with potential differences in APIs) between our kibana master branch and our next-release branch?
This feels like it could become a backport nightmare, doesn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@delvedor Can you elaborate on the typical changes between versions? If newer versions only change to support or remove new or deprecated ES functionality then this shouldn't cause any problems for us that aren't already caused by ES.
But if elasticsearch-js plans to make breaking changes to it's API signatures this adds an additional maintenance burden and we will have to migrate all code within the release timeframe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The client follows semantic versioning, so there will never be a breaking change between minor or patch releases, but there might be between majors.
Minor releases are always additive, in a generic minor release you will find new ES endpoints and some additional features of the client, for example, in the last 2/3 minors, client helpers have been added.
If the client needs to do a breaking change, which can be dropping the support for a specific version of Node, remove/change a configuration option, or drop an API, that will happen in a major release.
The only parts of the client that could have a breaking change between minors are the helpers and the type definitions, which are still experimental (even if they are stable and not expected to change unless there is a very good reason).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The client does prereleases as soon as there is a feature freeze, if you take a look at the published versions on npm you will see few rcs.