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

OpenTelemetry Tracing #26

Open
wants to merge 76 commits into
base: master
Choose a base branch
from
Open

OpenTelemetry Tracing #26

wants to merge 76 commits into from

Conversation

aMahanna
Copy link
Member

@aMahanna aMahanna commented Nov 23, 2023

Introduces OpenTelemetry Tracing as an optional mechanism to the adapter, along with a new Github Action Workflow (benchmark.yml) to compare the traces of pyg-adapter@master with future PR traces.

Usable with pip install adbpyg-adapter[tracing]

  • Minor method refactoring was required to take advantage of tracing decorators (no breaking changes)

Example:

from adbpyg_adapter import ADBPyG_Adapter
from adbpyg_adapter.tracing import create_tracer

from opentelemetry.exporter.otlp.proto.grpc.trace_exporter import OTLPSpanExporter

tracer = create_tracer(
    "adbpyg-adapter-test",
    enable_console_tracing=False,
    # e.g Export to local Jaeger Instance:
    span_exporters=[OTLPSpanExporter(endpoint="http://localhost:4317", insecure=True)],
)

adbpyg_adapter = ADBPyG_Adapter(..., tracer=tracer)

...

image
image

tracing = [
"opentelemetry-api==1.21.0",
"opentelemetry-sdk==1.21.0",
"opentelemetry-exporter-otlp-proto-grpc==1.21.0"
Copy link
Member Author

Choose a reason for hiding this comment

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

Need to revisit if we want opentelemetry-exporter-otlp-proto-http or opentelemetry-exporter-otlp-proto-grpc.

Alternatively we could have opentelemetry-exporter-otlp, which installs both

tests/conftest.py Dismissed Show dismissed Hide dismissed
@aMahanna aMahanna mentioned this pull request Dec 5, 2023
@aMahanna aMahanna mentioned this pull request Dec 5, 2023
@aMahanna aMahanna mentioned this pull request Dec 5, 2023
@arangoml arangoml deleted a comment from github-actions bot Dec 23, 2023
Copy link
Contributor

Benchmark (ddbd331)

{
  "pyg_to_arangodb": {
    "master_duration": 1470129,
    "branch_duration": 1462826,
    "improvement": "0%"
  },
  "arangodb_to_pyg": {
    "master_duration": 906241,
    "branch_duration": 1058905,
    "improvement": "-17%"
  }
}

See the full diff here

Copy link
Contributor

Benchmark (704cbba)

{
  "pyg_to_arangodb": {
    "master_duration": 1470129,
    "branch_duration": 1396778,
    "improvement": "5%"
  },
  "arangodb_to_pyg": {
    "master_duration": 906241,
    "branch_duration": 890910,
    "improvement": "2%"
  }
}

See the full diff here

Copy link
Contributor

Benchmark (3f35755)

{
  "pyg_to_arangodb": {
    "master_duration": 1424964,
    "branch_duration": 1567449,
    "improvement": "-10%"
  },
  "arangodb_to_pyg": {
    "master_duration": 836965,
    "branch_duration": 951386,
    "improvement": "-14%"
  }
}

See the full diff here

Copy link
Contributor

Benchmark (77b7a81)

{
  "pyg_to_arangodb": {
    "master_duration": 1424964,
    "branch_duration": 1390292,
    "improvement": "2%"
  },
  "arangodb_to_pyg": {
    "master_duration": 836965,
    "branch_duration": 874896,
    "improvement": "-5%"
  }
}

See the full diff here

Copy link
Contributor

github-actions bot commented Feb 9, 2024

Benchmark (d325fd4)

{
  "pyg_to_arangodb": {
    "master_duration": 1424964,
    "branch_duration": 1470840,
    "improvement": "-3%"
  },
  "arangodb_to_pyg": {
    "master_duration": 836965,
    "branch_duration": 1026411,
    "improvement": "-23%"
  }
}

See the full diff here

@aMahanna
Copy link
Member Author

aMahanna commented Feb 9, 2024

Looks like the changes made in process_adb_edge_type_df by 67378e8 resulted in doubling the duration for most calls of that span...

see the improvement field of every process_adb_edge_type_df span in the full diff

need to revisit

Copy link
Contributor

github-actions bot commented Feb 9, 2024

Benchmark (36e33e5)

{
  "pyg_to_arangodb": {
    "master_duration": 1424964,
    "branch_duration": 1361651,
    "improvement": "4%"
  },
  "arangodb_to_pyg": {
    "master_duration": 836965,
    "branch_duration": 884932,
    "improvement": "-6%"
  }
}

See the full diff here

@aMahanna
Copy link
Member Author

aMahanna commented Feb 9, 2024

update: I think I can disregard #26 (comment)

last benchmark shows similar performance.

I should probably run the benchmark more than once

Copy link
Contributor

Benchmark (91aed84)

{
  "pyg_to_arangodb": {
    "master_duration": 1424964,
    "branch_duration": 1507461,
    "improvement": "-6%"
  },
  "arangodb_to_pyg": {
    "master_duration": 836965,
    "branch_duration": 900063,
    "improvement": "-8%"
  }
}

See the full diff here

Copy link
Contributor

Benchmark (b8a19b3)

{
  "pyg_to_arangodb": {
    "master_duration": 1424964,
    "branch_duration": 1484173,
    "improvement": "-4%"
  },
  "arangodb_to_pyg": {
    "master_duration": 836965,
    "branch_duration": 889268,
    "improvement": "-6%"
  }
}

See the full diff here

Copy link
Contributor

Benchmark (7fb9f67)

{
  "pyg_to_arangodb": {
    "master_duration": 1424964,
    "branch_duration": 1380837,
    "improvement": "3%"
  },
  "arangodb_to_pyg": {
    "master_duration": 836965,
    "branch_duration": 888408,
    "improvement": "-6%"
  }
}

See the full diff here

Copy link
Contributor

Benchmark (7c67b70)

{
  "pyg_to_arangodb": {
    "master_duration": 1424964,
    "branch_duration": 1349974,
    "improvement": "5%"
  },
  "arangodb_to_pyg": {
    "master_duration": 836965,
    "branch_duration": 872216,
    "improvement": "-4%"
  }
}

See the full diff here

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.

1 participant