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

DSET-4559: Group By fields are part of SessionInfo #62

Merged
merged 4 commits into from
Nov 21, 2023

Conversation

martin-majlis-s1
Copy link
Collaborator

@martin-majlis-s1 martin-majlis-s1 commented Nov 15, 2023

Jira Link: https://sentinelone.atlassian.net/browse/DSET-4559

🥅 Goal

Existing integrations are supporting the specification of the server fields. It was not possible using this library. This PR allows to put arbitrary attributes into SessionInfo.

🛠️ Solution

DataSet UI is showing "Attributes" and "Server Fields". Server fields are taken from the sessionInfo parameter in the AddEvents API call. This behaviour is not documented there.

This PR is changing:

  • Adds logfile as another attribute, that is used for grouping. Now there is serverHost and logfile.
  • Attributes that are used for grouping are moved from Event.attrs to sessionInfo
  • Removes adding bundle_key among attributes.

🏫 Testing

To be able to test it:

  1. Modify opentelemetry-collector-contrib to use the modified library and do not use cached layers:
diff --git a/Makefile b/Makefile
index 30594c1b7a..6419c4d0db 100644
--- a/Makefile
+++ b/Makefile
@@ -208,7 +208,7 @@ run:
 docker-component: check-component
        GOOS=linux GOARCH=amd64 $(MAKE) $(COMPONENT)
        cp ./bin/$(COMPONENT)_linux_amd64 ./cmd/$(COMPONENT)/$(COMPONENT)
-       docker build -t $(COMPONENT) ./cmd/$(COMPONENT)/
+       docker build --no-cache -t $(COMPONENT) ./cmd/$(COMPONENT)/
        rm ./cmd/$(COMPONENT)/$(COMPONENT)

 .PHONY: check-component
diff --git a/cmd/otelcontribcol/go.mod b/cmd/otelcontribcol/go.mod
index 73c2146146..fda73dc204 100644
--- a/cmd/otelcontribcol/go.mod
+++ b/cmd/otelcontribcol/go.mod
@@ -1149,3 +1149,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator
 replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/skywalking => ../../pkg/translator/skywalking

 replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/collectd => ../../internal/collectd
+
+replace github.com/scalyr/dataset-go => ../../../dataset-go
\ No newline at end of file
diff --git a/exporter/datasetexporter/go.mod b/exporter/datasetexporter/go.mod
index c8181b324d..2b15007d46 100644
--- a/exporter/datasetexporter/go.mod
+++ b/exporter/datasetexporter/go.mod
@@ -67,3 +67,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest
 replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil => ../../pkg/pdatautil

 replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden
+
+replace github.com/scalyr/dataset-go => ../../../dataset-go
diff --git a/go.mod b/go.mod
index b845553002..a0ead72a6b 100644
--- a/go.mod
+++ b/go.mod
@@ -1133,3 +1133,6 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/receiver/azure
 replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ./pkg/golden

 replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/collectd => ./internal/collectd
+
+
+replace github.com/scalyr/dataset-go => ../dataset-go/
  1. Use make docker-otelcontribcol to build docker images
  2. Modify opentelemetry-demo to use built image instead of the official one:
diff --cc docker-compose.yml
index 001f7c8,f00e675..0000000
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@@ -646,16 -644,18 +646,19 @@@ services

    # OpenTelemetry Collector
    otelcol:
-     image: otel/opentelemetry-collector-contrib:0.86.0
+     image: otelcontribcol:latest
  1. Modify src/otelcollector/otelcol-config.yml to include datasetexporter configuration.
  2. Run it - docker compose up --abort-on-container-exit

@codecov-commenter
Copy link

codecov-commenter commented Nov 16, 2023

Codecov Report

Merging #62 (8eebfd0) into main (5f915d0) will decrease coverage by 0.29%.
The diff coverage is 97.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
- Coverage   76.12%   75.84%   -0.29%     
==========================================
  Files          12       11       -1     
  Lines        1713     1763      +50     
==========================================
+ Hits         1304     1337      +33     
- Misses        340      359      +19     
+ Partials       69       67       -2     
Files Coverage Δ
pkg/api/add_events/add_events.go 80.60% <ø> (ø)
pkg/client/client.go 87.36% <100.00%> (-0.97%) ⬇️
pkg/config/config.go 87.76% <100.00%> (+1.09%) ⬆️
pkg/client/add_events.go 81.40% <96.25%> (-0.33%) ⬇️

pkg/client/add_events.go Outdated Show resolved Hide resolved
@tomaz-s1
Copy link
Collaborator

For the roll out purposes since in theory the change is "breaking", do you think we need a feature flag (it can also be on by default) so user can turn it off in case something goes wrong? Or you think no feature flag / config option is needed?

pkg/client/add_events.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@tomaz-s1 tomaz-s1 left a comment

Choose a reason for hiding this comment

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

Minor comment / question on debug=true test case, besides that, LGTM.

@martin-majlis-s1 martin-majlis-s1 added this pull request to the merge queue Nov 21, 2023
Merged via the queue into main with commit 29d4782 Nov 21, 2023
10 checks passed
@martin-majlis-s1 martin-majlis-s1 deleted the DSET-4559-move-group-by-into-session-info branch November 21, 2023 09:39
codeboten pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this pull request Nov 28, 2023
Upgrade to new version of the library.

This PR is implementing following issues:

* #27650 - metrics are not collected via open telemetry, so they can be
monitored. It's better version of the previous PR #27487 which was not
working.
* #27652 - it's configurable with the `debug` option whether
`session_key` is included or not

Other change is that fields that are specified as part of the `group_by`
configuration are now transferred as part of the session info.

**Link to tracking Issue:** #27650, #27652

**Testing:** 

1. Build docker image - make docker-otelcontribcol
2. Checkout https://github.com/open-telemetry/opentelemetry-demo
3. Update configuration in `docker-compose.yaml` and in the
`src/otelcollector/otelcol-config.yml`:
* In `docker-compose.yaml` switch image to the newly build one in step 1
* In `docker-compose.yaml` enable feature gate for collecting metrics -
`--feature-gates=telemetry.useOtelForInternalMetrics`
* In `src/otelcollector/otelcol-config.yml` enable metrics scraping by
prometheus
* In `src/otelcollector/otelcol-config.yml` add configuration for
dataset
```diff
diff --git a/docker-compose.yml b/docker-compose.yml
index 001f7c8..d7edd0d 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -646,14 +646,16 @@ services:

   # OpenTelemetry Collector
   otelcol:
-    image: otel/opentelemetry-collector-contrib:0.86.0
+    image: otelcontribcol:latest
     container_name: otel-col
     deploy:
       resources:
         limits:
           memory: 125M
     restart: unless-stopped
-    command: [ "--config=/etc/otelcol-config.yml", "--config=/etc/otelcol-config-extras.yml" ]
+    command: [ "--config=/etc/otelcol-config.yml", "--config=/etc/otelcol-config-extras.yml", "--feature-gates=telemetry.useOtelForInternalMetrics" ]
     volumes:
       - ./src/otelcollector/otelcol-config.yml:/etc/otelcol-config.yml
       - ./src/otelcollector/otelcol-config-extras.yml:/etc/otelcol-config-extras.yml
diff --git a/src/otelcollector/otelcol-config.yml b/src/otelcollector/otelcol-config.yml
index f2568ae..9944562 100644
--- a/src/otelcollector/otelcol-config.yml
+++ b/src/otelcollector/otelcol-config.yml
@@ -15,6 +15,14 @@ receivers:
     targets:
       - endpoint: http://frontendproxy:${env:ENVOY_PORT}

+  prometheus:
+    config:
+      scrape_configs:
+        - job_name: 'otel-collector'
+          scrape_interval: 5s
+          static_configs:
+            - targets: ['0.0.0.0:8888']
+
 exporters:
   debug:
   otlp:
@@ -29,6 +37,22 @@ exporters:
     endpoint: "http://prometheus:9090/api/v1/otlp"
     tls:
       insecure: true
+  logging:
+  dataset:
+    api_key: API_KEY
+    dataset_url: https://SERVER.scalyr.com
+    debug: true
+    buffer:
+      group_by:
+        - resource_name
+        - resource_type
+    logs:
+      export_resource_info_on_event: true
+    server_host:
+      server_host: Martin
+      use_hostname: false
+  dataset/aaa:
+    api_key: API_KEY
+    dataset_url: https://SERVER.scalyr.com
+    debug: true
+    buffer:
+      group_by:
+        - resource_name
+        - resource_type
+    logs:
+      export_resource_info_on_event: true
+    server_host:
+      server_host: MartinAAA
+      use_hostname: false

 processors:
   batch:
@@ -47,6 +71,11 @@ processors:
           - set(description, "") where name == "queueSize"
           # FIXME: remove when this issue is resolved: open-telemetry/opentelemetry-python-contrib#1958
           - set(description, "") where name == "http.client.duration"
+  attributes:
+    actions:
+      - key: otel.demo
+        value: 29446
+        action: upsert

 connectors:
   spanmetrics:
@@ -55,13 +84,13 @@ service:
   pipelines:
     traces:
       receivers: [otlp]
-      processors: [batch]
-      exporters: [otlp, debug, spanmetrics]
+      processors: [batch, attributes]
+      exporters: [otlp, debug, spanmetrics, dataset, dataset/aaa]
     metrics:
-      receivers: [httpcheck/frontendproxy, otlp, spanmetrics]
+      receivers: [httpcheck/frontendproxy, otlp, spanmetrics, prometheus]
       processors: [filter/ottl, transform, batch]
       exporters: [otlphttp/prometheus, debug]
     logs:
       receivers: [otlp]
-      processors: [batch]
-      exporters: [otlp/logs, debug]
+      processors: [batch, attributes]
+      exporters: [otlp/logs, debug, dataset, dataset/aaa]
```
4. Run the demo - `docker compose up --abort-on-container-exit`
5. Check, that metrics are in Grafana -
http://localhost:8080/grafana/explore?
<img width="838" alt="Screenshot 2023-11-27 at 12 29 29"
src="https://github.com/open-telemetry/opentelemetry-collector-contrib/assets/122797378/43d365dd-37d8-4528-b768-1d7f0ac34989">
6. Check some metrics
![Screenshot 2023-11-22 at 14 06
56](https://github.com/open-telemetry/opentelemetry-collector-contrib/assets/122797378/81306486-eb5e-49b1-87ed-25d1eb8afcf8)
<img width="1356" alt="Screenshot 2023-11-27 at 12 59 10"
src="https://github.com/open-telemetry/opentelemetry-collector-contrib/assets/122797378/34c36e45-850e-4e74-a18a-0a54ce97cbd3">
7. Check that data are available in dataset ![Screenshot 2023-11-22 at
13 33
50](https://github.com/open-telemetry/opentelemetry-collector-contrib/assets/122797378/77cb2f31-be14-463b-91a7-fd10f8dbfe3a)

**Documentation:** 

**Library changes:**
* Group By & Debug - scalyr/dataset-go#62
* Metrics  - scalyr/dataset-go#61

---------

Co-authored-by: Andrzej Stencel <[email protected]>
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.

3 participants