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

Fix tracing to continue from existing trace created by grpc client #245

Merged
merged 19 commits into from
Sep 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
fcd11ce
Fix tracing to continue from existing trace created by grpc client
Sep 9, 2019
d92587c
Update python SDK with updated protos in Feast 0.1.x
davidheryanto Sep 10, 2019
e05cd44
Add __init__ file in feast/types to fix module not found error
davidheryanto Sep 10, 2019
fae0041
Add support to configure import job metrics in Feast Core
davidheryanto Sep 10, 2019
e16fd0f
Fix CoreApplicationWithNoWarehouseTest
Sep 10, 2019
a424fc1
Update charts
davidheryanto Sep 10, 2019
ebc50dc
Add lazy annotation in getBigQueryTrainingDatasetTemplater in Trainin…
davidheryanto Sep 10, 2019
9210cf7
Use provided values for INGESTION_METRICS_ENABLED
davidheryanto Sep 10, 2019
1634a96
Add Python mypy definition
davidheryanto Sep 10, 2019
3173740
Increase version of google-cloud-storage in python sdk
davidheryanto Sep 10, 2019
cba6139
Update docs for influx_db_url field in ImportJobSpecs proto
davidheryanto Sep 11, 2019
9f5aa65
Write feature metrics in fixed window for better performance
davidheryanto Sep 12, 2019
d78b01d
Update default image in Helm chart
davidheryanto Sep 12, 2019
5ed4380
Write feature metrics only if the job type is streaming
davidheryanto Sep 12, 2019
94905f9
Set transform name for writing feature metrics to Influx DB
davidheryanto Sep 12, 2019
c4da34c
Update chart, making resources limit optional
davidheryanto Sep 12, 2019
a4497dc
Fix the check for unbounded sources
davidheryanto Sep 12, 2019
06f1514
Fix typo
davidheryanto Sep 19, 2019
2b79d47
Update default Feast image tag used in the chart
davidheryanto Sep 19, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions charts/feast/Chart.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
apiVersion: v1
appVersion: "0.1.4"
appVersion: "0.1.7"
description: A Helm chart to install Feast on kubernetes
name: feast
version: 0.1.2
version: 0.1.4
5 changes: 4 additions & 1 deletion charts/feast/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ Components that Feast supports, but this installation will not include are:
- Note that if you do not provision a metrics store, feast will only retain the latest metrics from your jobs.
- [Jaeger tracing](www.jaegertracing.io) for serving performance.
- Set `serving.jaeger.enabled` to `true`, and configure the following parameters:
- `serving.jaeger.host`
- `serving.jaeger.port`
- `serving.jaeger.options.samplerType`
- `serving.jaeger.options.samplerParam`
Expand All @@ -72,6 +71,10 @@ The following table lists the configurable parameters of the Feast chart and the
| `core.jobs.options` | additional options to be provided to the beam job. Should be a char escaped json k-v object | {} |
| `core.jobs.runner` | beam job runner - one of `DirectRunner`, `FlinkRunner` or `DataflowRunner` | DirectRunner |
| `core.jobs.workspace` | workspace path for ingestion jobs, used for separate job workspaces to share importJobSpecs.yaml with ingestion and for writing errors to if no default errors store is configured | nil |
| `core.jobs.writeFeatureMetricsToInfluxDb` | specifies whether Feast import job will write feature metrics (such as feature lag and values summaries) to Influx DB for monitoring and alert purpose | false |
| `core.jobs.influxDbUrl` | Influx DB url e.g. http://localhost:8086 (required if `core.jobs.writeFeatureMetricsToInfluxDb = true`) | |
| `core.jobs.influxDbDatabase` | Influx DB database name (required if `core.jobs.writeFeatureMetricsToInfluxDb = true`) | |
| `core.jobs.influxDbMeasurement` | Influx DB [measurement name](https://docs.influxdata.com/influxdb/v1.7/concepts/key_concepts/#measurement) (required if `core.jobs.writeFeatureMetricsToInfluxDb = true`) | |
| `core.replicaCount` | core deployment replica count | 3 |
| `core.resources.limits.cpu` | core cpu limits | 1 |
| `core.resources.limits.memory` | core memory limits | 2G |
Expand Down
19 changes: 13 additions & 6 deletions charts/feast/templates/core-deploy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,7 @@ spec:
timeoutSeconds: 3
failureThreshold: {{ .Values.core.readinessProbe.failureThreshold }}
resources:
requests:
cpu: {{ .Values.core.resources.requests.cpu }}
memory: {{ .Values.core.resources.requests.memory }}
limits:
cpu: {{ .Values.core.resources.limits.cpu }}
memory: {{ .Values.core.resources.limits.memory }}
{{ toYaml .Values.core.resources | indent 10 }}
{{- if .Values.serviceAccount }}
volumeMounts:
- name: "{{ .Values.serviceAccount.name }}"
Expand Down Expand Up @@ -98,6 +93,18 @@ spec:
value: "{{ .Values.core.jobs.monitoring.period }}"
- name: JOB_MONITOR_INITIAL_DELAY_MS
value: "{{ .Values.core.jobs.monitoring.initialDelay }}"

{{- if .Values.core.jobs.writeFeatureMetricsToInfluxDb }}
- name: INGESTION_METRICS_ENABLED
value: "{{ .Values.core.jobs.writeFeatureMetricsToInfluxDb }}"
- name: INGESTION_METRICS_INFLUX_URL
value: {{ .Values.core.jobs.influxDbUrl }}
- name: INGESTION_METRICS_INFLUX_DB_NAME
value: {{ .Values.core.jobs.influxDbDatabase }}
- name: INGESTION_METRICS_INFLUX_DB_MEASUREMENT
value: {{ .Values.core.jobs.influxDbMeasurement }}
{{- end }}

{{- if .Values.store }}
{{- if .Values.store.serving }}
- name: STORE_SERVING_TYPE
Expand Down
11 changes: 4 additions & 7 deletions charts/feast/templates/serving-deploy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,7 @@ spec:
timeoutSeconds: 3
failureThreshold: {{ .Values.serving.readinessProbe.failureThreshold }}
resources:
requests:
cpu: "{{ .Values.serving.resources.requests.cpu }}"
memory: "{{ .Values.serving.resources.requests.memory }}"
limits:
cpu: "{{ .Values.serving.resources.limits.cpu }}"
memory: "{{ .Values.serving.resources.limits.memory }}"
{{ toYaml .Values.serving.resources | indent 10 }}
env:
- name: FEAST_SERVING_HTTP_PORT
value: "{{ .Values.serving.service.http.targetPort }}"
Expand Down Expand Up @@ -96,7 +91,9 @@ spec:
- name: JAEGER_ENABLED
value: "{{ .Values.serving.jaeger.enabled }}"
- name: JAEGER_AGENT_HOST
value: "{{ .Values.serving.jaeger.host }}"
valueFrom:
fieldRef:
fieldPath: status.hostIP
- name: JAEGER_AGENT_PORT
value: "{{ .Values.serving.jaeger.port }}"
- name: JAEGER_SAMPLER_TYPE
Expand Down
36 changes: 27 additions & 9 deletions charts/feast/values.yaml
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
---
# [required]
# global.postgresql.secretName is an existing Kubernetes secret name containing Postgresql password
# The secret needs to have this key: postgresql-password
#
# Example of how to generate this secret:
# kubectl create secret generic <feast_helm_release_name>-postgresql \
# --from-literal=postgresql-password=<postgresql_password>
#
# With the example above the secretName will be <feast_helm_release_name>-postgresql
#
# global:
# postgresql:
# secretName: <existing_kubernetes_secret_name_containing_postgresql_credentials>

core:
projectId: "gcp-project-id"
image:
pullPolicy: IfNotPresent
registry: gcr.io/kf-feast
repository: feast-core
tag: "0.1.4"
tag: "0.1.7"
replicaCount: 1
resources:
limits:
cpu: 4
memory: 6G
requests:
cpu: 1
memory: 2G
Expand Down Expand Up @@ -38,6 +48,14 @@ core:
monitoring:
period: 5000
initialDelay: 60000
# writeFeatureMetricsToInfluxDb specifies whether Feast import job will write feature metrics (such as feature lag and values summaries) to Influx DB for monitoring and alert purpose
writeFeatureMetricsToInfluxDb: false
# influxDbUrl, influxDbDatabase and influxDbMeasurement sets the Influx DB configuration where Feast import job will write the feature metrics
# Uncomment the following 3 fields if writeFeatureMetricsToInfluxDb = true
#
# influxDbUrl: http://localhost:8086
# influxDbDatabase: influx_db_database
# influxDbMeasurement: influx_db_measurement
trainingDatasetPrefix: "fs"
# logType: JSON
livenessProbe:
Expand Down Expand Up @@ -84,12 +102,9 @@ serving:
pullPolicy: IfNotPresent
registry: gcr.io/kf-feast
repository: feast-serving
tag: "0.1.4"
tag: "0.1.7"
replicaCount: 1
resources:
limits:
cpu: 2
memory: 4G
requests:
cpu: 1
memory: 1G
Expand All @@ -109,6 +124,9 @@ serving:
# loadBalancerSourceRanges: ["10.0.0.0/8"]
jaeger:
enabled: false
# options:
# samplerType: constants
# samplerParam: 1
livenessProbe:
initialDelaySeconds: 120
failureThreshold: 3
Expand Down
13 changes: 13 additions & 0 deletions core/src/main/java/feast/core/config/ImportJobMetricsConfig.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package feast.core.config;

import lombok.AllArgsConstructor;
import lombok.Getter;

@Getter
@AllArgsConstructor
public class ImportJobMetricsConfig {
private final boolean ingestionMetricsEnabled;
private final String influxDbUrl;
private final String influxDbName;
private final String influxDbMeasurementName;
}
13 changes: 11 additions & 2 deletions core/src/main/java/feast/core/config/InstrumentationConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,17 @@
@Configuration
public class InstrumentationConfig {
@Bean
public StatsDClient getStatsDClient(@Value("${statsd.host}") String host,
@Value("${statsd.port}") int port) {
public StatsDClient getStatsDClient(
@Value("${statsd.host}") String host, @Value("${statsd.port}") int port) {
return new NonBlockingStatsDClient("feast_core", host, port);
}

@Bean
public ImportJobMetricsConfig getImportJobMetricsConfig(
@Value("${ingestion.metrics.enabled}") boolean enabled,
@Value("${ingestion.metrics.influxUrl}") String influxDbUrl,
@Value("${ingestion.metrics.dbName}") String influxDbName,
@Value("${ingestion.metrics.dbMeasurement}") String influxDbMeasurementName) {
return new ImportJobMetricsConfig(enabled, influxDbUrl, influxDbName, influxDbMeasurementName);
}
}
3 changes: 2 additions & 1 deletion core/src/main/java/feast/core/config/TrainingConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Lazy;
import org.springframework.core.io.ClassPathResource;
import org.springframework.core.io.Resource;

Expand All @@ -23,7 +24,7 @@ public class TrainingConfig {

@Bean
public BigQueryDatasetTemplater getBigQueryTrainingDatasetTemplater(
StorageSpecs storageSpecs, FeatureInfoRepository featureInfoRepository) throws IOException {
@Lazy StorageSpecs storageSpecs, FeatureInfoRepository featureInfoRepository) throws IOException {
Resource resource = new ClassPathResource("templates/bq_training.tmpl");
InputStream resourceInputStream = resource.getInputStream();
String tmpl = CharStreams.toString(new InputStreamReader(resourceInputStream, Charsets.UTF_8));
Expand Down
12 changes: 10 additions & 2 deletions core/src/main/java/feast/core/service/JobManagementService.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.protobuf.util.JsonFormat;
import feast.core.JobServiceProto.JobServiceTypes.JobDetail;
import feast.core.config.ImportJobDefaults;
import feast.core.config.ImportJobMetricsConfig;
import feast.core.config.StorageConfig.StorageSpecs;
import feast.core.dao.JobInfoRepository;
import feast.core.dao.MetricsRepository;
Expand Down Expand Up @@ -73,6 +74,7 @@ public class JobManagementService {
private static final String JOB_PREFIX_DEFAULT = "feastimport";
private static final String UNKNOWN_EXT_JOB_ID = "";
private static final String IMPORT_JOB_SPECS_FILENAME = "importJobSpecs.yaml";
private ImportJobMetricsConfig metricsConfig;

private JobInfoRepository jobInfoRepository;
private MetricsRepository metricsRepository;
Expand All @@ -88,13 +90,15 @@ public JobManagementService(
JobManager jobManager,
ImportJobDefaults defaults,
SpecService specService,
StorageSpecs storageSpecs) {
StorageSpecs storageSpecs,
ImportJobMetricsConfig metricsConfig) {
this.jobInfoRepository = jobInfoRepository;
this.metricsRepository = metricsRepository;
this.jobManager = jobManager;
this.defaults = defaults;
this.specService = specService;
this.storageSpecs = storageSpecs;
this.metricsConfig = metricsConfig;
}

public void writeImportJobSpecs(ImportJobSpecs importJobSpecs, Path workspace) {
Expand Down Expand Up @@ -132,7 +136,11 @@ private ImportJobSpecs buildImportJobSpecs(ImportSpec importSpec, String jobId)
.setJobId(jobId)
.setImportSpec(importSpec)
.addAllEntitySpecs(entitySpecs)
.addAllFeatureSpecs(featureSpecs);
.addAllFeatureSpecs(featureSpecs)
.setWriteFeatureMetricsToInfluxDb(metricsConfig.isIngestionMetricsEnabled())
.setInfluxDbUrl(metricsConfig.getInfluxDbUrl())
.setInfluxDbDatabase(metricsConfig.getInfluxDbName())
.setInfluxDbMeasurement(metricsConfig.getInfluxDbMeasurementName());
if (storageSpecs.getServingStorageSpec() != null) {
importJobSpecsBuilder.setServingStorageSpec(storageSpecs.getServingStorageSpec());
}
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,8 @@ management.metrics.export.simple.enabled=false
management.metrics.export.statsd.enabled=true
management.metrics.export.statsd.host=${STATSD_HOST:localhost}
management.metrics.export.statsd.port=${STATSD_PORT:8125}

ingestion.metrics.enabled=${INGESTION_METRICS_ENABLED:false}
ingestion.metrics.influxUrl=${INGESTION_METRICS_INFLUX_URL:}
ingestion.metrics.dbName=${INGESTION_METRICS_INFLUX_DB_NAME:}
ingestion.metrics.dbMeasurement=${INGESTION_METRICS_INFLUX_DB_MEASUREMENT:}
10 changes: 9 additions & 1 deletion core/src/test/java/feast/core/CoreApplicationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@
"feast.store.warehouse.options={\"path\":\"/tmp/foobar\"}",
"feast.store.serving.type=redis",
"feast.store.serving.options={\"host\":\"localhost\",\"port\":1234}",
"feast.store.errors.type=stderr"
"feast.store.errors.type=stderr",
"ingestion.metrics.enabled=true",
"ingestion.metrics.influxUrl=localhost",
"ingestion.metrics.dbName=db",
"ingestion.metrics.dbMeasurement=measurement"
})
@DirtiesContext
public class CoreApplicationTest {
Expand Down Expand Up @@ -138,6 +142,10 @@ public void test_withProperties_systemServingAndWarehouseStoresRegistered() thro
.setId(DEFAULT_WAREHOUSE_ID)
.setType("file.json")
.putOptions("path", "/tmp/foobar"))
.setWriteFeatureMetricsToInfluxDb(true)
.setInfluxDbUrl("localhost")
.setInfluxDbDatabase("db")
.setInfluxDbMeasurement("measurement")
.build(), args.get(0));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@
"spring.datasource.url=jdbc:h2:mem:testdb",
"feast.store.warehouse.type=file.json",
"feast.store.warehouse.options={\"path\":\"/tmp/foobar\"}",
"feast.store.errors.type=stderr"
"feast.store.errors.type=stderr",
"ingestion.metrics.enabled=true",
"ingestion.metrics.influxUrl=localhost",
"ingestion.metrics.dbName=db",
"ingestion.metrics.dbMeasurement=measurement"
})
@DirtiesContext
public class CoreApplicationWithNoServingTest {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@
"spring.datasource.url=jdbc:h2:mem:testdb",
"feast.store.serving.type=redis",
"feast.store.serving.options={\"host\":\"localhost\",\"port\":1234}",
"feast.store.errors.type=stderr"
"feast.store.errors.type=stderr",
"ingestion.metrics.enabled=true",
"ingestion.metrics.influxUrl=localhost",
"ingestion.metrics.dbName=db",
"ingestion.metrics.dbMeasurement=measurement"
})
@DirtiesContext
public class CoreApplicationWithNoWarehouseTest {
Expand Down Expand Up @@ -132,6 +136,10 @@ public void test_withProperties_systemServingAndWarehouseStoresRegistered() thro
.setId(DEFAULT_SERVING_ID)
.setType("redis")
.putOptions("host", "localhost").putOptions("port", "1234"))
.setWriteFeatureMetricsToInfluxDb(true)
.setInfluxDbUrl("localhost")
.setInfluxDbDatabase("db")
.setInfluxDbMeasurement("measurement")
.build(), args.get(0));
}

Expand Down
Loading